From 43c24d3ee2168768c9f21fdb8c7c8bb982f5b50a Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Tue, 10 Feb 2026 09:15:50 -0800 Subject: [PATCH 01/19] init payload processing --- beacon_node/beacon_chain/src/beacon_chain.rs | 18 + .../beacon_chain/src/block_verification.rs | 2 +- .../beacon_chain/src/execution_payload.rs | 1 + beacon_node/beacon_chain/src/lib.rs | 2 + .../src/payload_envelope_verification.rs | 419 ++++++++++++++++++ .../payload_envelope_verification_types.rs | 31 ++ 6 files changed, 472 insertions(+), 1 deletion(-) create mode 100644 beacon_node/beacon_chain/src/payload_envelope_verification.rs create mode 100644 beacon_node/beacon_chain/src/payload_envelope_verification_types.rs diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index ec79153785..a59508a37a 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1147,6 +1147,24 @@ impl BeaconChain { } } + + /// Returns the full block at the given root, if it's available in the database. + /// + /// Should always return a full block for pre-merge and post-gloas blocks. + pub fn get_full_block( + &self, + block_root: &Hash256, + ) -> Result>, Error> { + match self.store.try_get_full_block(block_root)? { + Some(DatabaseBlock::Full(block)) => Ok(Some(block)), + Some(DatabaseBlock::Blinded(_)) => { + // TODO(gloas) this should error out + todo!() + } + None => Ok(None), + } + } + /// Returns the block at the given root, if any. /// /// ## Errors diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 9bb6757341..9d1b8e0d3d 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -718,7 +718,7 @@ pub struct SignatureVerifiedBlock { } /// Used to await the result of executing payload with an EE. -type PayloadVerificationHandle = JoinHandle>>; +pub type PayloadVerificationHandle = JoinHandle>>; /// A wrapper around a `SignedBeaconBlock` that indicates that this block is fully verified and /// ready to import into the `BeaconChain`. The validation includes: diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index bdf3ab9594..370f044c7a 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -7,6 +7,7 @@ //! So, this module contains functions that one might expect to find in other crates, but they live //! here for good reason. +use crate::payload_envelope_verification::EnvelopeError; use crate::{ BeaconChain, BeaconChainError, BeaconChainTypes, BlockError, BlockProductionError, ExecutionPayloadError, diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index e77739e2d5..7aa64deb1c 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -42,6 +42,8 @@ pub mod observed_block_producers; pub mod observed_data_sidecars; pub mod observed_operations; mod observed_slashable; +pub mod payload_envelope_verification; +pub mod payload_envelope_verification_types; pub mod persisted_beacon_chain; pub mod persisted_custody; mod persisted_fork_choice; diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification.rs b/beacon_node/beacon_chain/src/payload_envelope_verification.rs new file mode 100644 index 0000000000..b22acd69fb --- /dev/null +++ b/beacon_node/beacon_chain/src/payload_envelope_verification.rs @@ -0,0 +1,419 @@ +//! The incremental processing steps (e.g., signatures verified but not the state transition) is +//! represented as a sequence of wrapper-types around the block. There is a linear progression of +//! types, starting at a `SignedBeaconBlock` and finishing with a `Fully VerifiedBlock` (see +//! diagram below). +//! +//! ```ignore +//! START +//! | +//! ▼ +//! SignedExecutionPayloadEnvelope +//! | +//! |--------------- +//! | | +//! | ▼ +//! | GossipVerifiedEnvelope +//! | | +//! |--------------- +//! | +//! ▼ +//! ExecutionPendingEnvelope +//! | +//! await +//! | +//! ▼ +//! END +//! +//! ``` + +use crate::NotifyExecutionLayer; +use crate::block_verification::{PayloadVerificationHandle, PayloadVerificationOutcome}; +use crate::payload_envelope_verification_types::{EnvelopeImportData, MaybeAvailableEnvelope}; +use crate::execution_payload::PayloadNotifier; +use crate::{BeaconChain, BeaconChainError, BeaconChainTypes}; +use educe::Educe; +use slot_clock::SlotClock; +use state_processing::envelope_processing::{EnvelopeProcessingError, process_execution_payload_envelope}; +use state_processing::{BlockProcessingError, VerifySignatures}; +use std::sync::Arc; +use tracing::{debug, instrument}; +use types::{ + BeaconState, BeaconStateError, EthSpec, ExecutionBlockHash, Hash256, SignedBeaconBlock, + SignedExecutionPayloadEnvelope, Slot, +}; + +#[derive(Debug, Clone)] +pub enum EnvelopeError { + /// The envelope's block root is unknown. + BlockRootUnknown { + block_root: Hash256, + }, + /// The signature is invalid. + BadSignature, + /// The builder index doesn't match the committed bid + BuilderIndexMismatch { + committed_bid: u64, + envelope: u64, + }, + // The envelope slot doesn't match the block + SlotMismatch { + block: Slot, + envelope: Slot, + }, + // The validator index is unknown + UnknownValidator { + builder_index: u64, + }, + // The block hash doesn't match the committed bid + BlockHashMismatch { + committed_bid: ExecutionBlockHash, + envelope: ExecutionBlockHash, + }, + // Some Beacon Chain Error + BeaconChainError(Arc), + // Some Beacon State error + BeaconStateError(BeaconStateError), + // Some BlockProcessingError (for electra operations) + BlockProcessingError(BlockProcessingError), + // Some EnvelopeProcessingError + EnvelopeProcessingError(EnvelopeProcessingError), +} + +impl From for EnvelopeError { + fn from(e: BeaconChainError) -> Self { + EnvelopeError::BeaconChainError(Arc::new(e)) + } +} + +impl From for EnvelopeError { + fn from(e: BeaconStateError) -> Self { + EnvelopeError::BeaconStateError(e) + } +} + +/// Pull errors up from EnvelopeProcessingError to EnvelopeError +impl From for EnvelopeError { + fn from(e: EnvelopeProcessingError) -> Self { + match e { + EnvelopeProcessingError::BadSignature => EnvelopeError::BadSignature, + EnvelopeProcessingError::BeaconStateError(e) => EnvelopeError::BeaconStateError(e), + EnvelopeProcessingError::BlockHashMismatch { + committed_bid, + envelope, + } => EnvelopeError::BlockHashMismatch { + committed_bid, + envelope, + }, + EnvelopeProcessingError::BlockProcessingError(e) => { + EnvelopeError::BlockProcessingError(e) + } + e => EnvelopeError::EnvelopeProcessingError(e), + } + } +} + +/// This snapshot is to be used for verifying a envelope of the block. +#[derive(Debug, Clone)] +pub struct EnvelopeProcessingSnapshot { + /// This state is equivalent to the `self.beacon_block.state_root()` before applying the envelope. + pub pre_state: BeaconState, + pub state_root: Hash256, + pub beacon_block_root: Hash256, +} + +#[allow(clippy::type_complexity)] +#[instrument(skip_all, level = "debug", fields(beacon_block_root = %envelope.beacon_block_root()))] +fn load_snapshot( + envelope: &SignedExecutionPayloadEnvelope, + chain: &BeaconChain, +) -> Result, EnvelopeError> { + // Reject any block if its block is not known to fork choice. + // + // A block that is not in fork choice is either: + // + // - Not yet imported: we should reject this block because we should only import a child + // envelope after its parent has been fully imported. + // - Pre-finalized: if the block is _prior_ to finalization, we should ignore the envelope + // because it will revert finalization. Note that the finalized block is stored in fork + // choice, so we will not reject any child of the finalized block (this is relevant during + // genesis). + + let fork_choice_read_lock = chain.canonical_head.fork_choice_read_lock(); + let beacon_block_root = envelope.beacon_block_root(); + let Some(proto_beacon_block) = fork_choice_read_lock.get_block(&beacon_block_root) else { + return Err(EnvelopeError::BlockRootUnknown { + block_root: beacon_block_root, + }); + }; + drop(fork_choice_read_lock); + + // TODO(EIP-7732): add metrics here + + let block_state_root = proto_beacon_block.state_root; + // We can use `get_hot_state` here rather than `get_advanced_hot_state` because the envelope + // must be from the same slot as its block (so no advance is required). + let cache_state = true; + let state = chain + .store + .get_hot_state(&block_state_root, cache_state) + .map_err(|e| EnvelopeError::BeaconChainError(Arc::new(e.into())))? + .ok_or_else(|| { + BeaconChainError::DBInconsistent(format!( + "Missing state for envelope block {block_state_root:?}", + )) + })?; + + Ok(EnvelopeProcessingSnapshot { + pre_state: state, + state_root: block_state_root, + beacon_block_root, + }) +} + +/// A wrapper around a `SignedExecutionPayloadEnvelope` that indicates it has been approved for re-gossiping on +/// the p2p network. +#[derive(Educe)] +#[educe(Debug(bound = "T: BeaconChainTypes"))] +pub struct GossipVerifiedEnvelope { + pub signed_envelope: Arc>, + pub block: Arc>, + pub snapshot: Option>>, +} + +impl GossipVerifiedEnvelope { + pub fn new( + signed_envelope: Arc>, + chain: &BeaconChain, + ) -> Result { + let envelope = &signed_envelope.message; + let payload = &envelope.payload; + let beacon_block_root = envelope.beacon_block_root; + + // Check that we've seen the beacon block for this envelope and that it passes validation. + // TODO(EIP-7732): We need a block status table in order to differentiate between: + // + // 1. Blocks we haven't seen (IGNORE), and + // 2. Blocks we've seen that are invalid (REJECT). + // + // Presently these two cases are conflated. + let fork_choice_read_lock = chain.canonical_head.fork_choice_read_lock(); + let Some(proto_block) = fork_choice_read_lock.get_block(&beacon_block_root) else { + return Err(EnvelopeError::BlockRootUnknown { + block_root: beacon_block_root, + }); + }; + drop(fork_choice_read_lock); + + // TODO(EIP-7732): check that we haven't seen another valid `SignedExecutionPayloadEnvelope` + // for this block root from this builder - envelope status table check + + // TODO(EIP-7732): this could be obtained from the ProtoBlock instead of the DB + // but this means the ProtoBlock needs to include something like the ExecutionBid + // will need to answer this question later. + let block = chain + .get_full_block(&beacon_block_root)? + .ok_or_else(|| { + EnvelopeError::from(BeaconChainError::MissingBeaconBlock(beacon_block_root)) + }) + .map(Arc::new)?; + let execution_bid = &block + .message() + .body() + .signed_execution_payload_bid()? + .message; + + // TODO(EIP-7732): Gossip rules for the beacon block contain the following: + // https://github.com/ethereum/consensus-specs/blob/master/specs/phase0/p2p-interface.md#beacon_block + // [IGNORE] The block is not from a future slot (with a MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance) + // [IGNORE] The block is from a slot greater than the latest finalized slot + // should these kinds of checks be included for envelopes as well? + + // check that the slot of the envelope matches the slot of the parent block + if envelope.slot != block.slot() { + return Err(EnvelopeError::SlotMismatch { + block: block.slot(), + envelope: envelope.slot, + }); + } + + // builder index matches committed bid + if envelope.builder_index != execution_bid.builder_index { + return Err(EnvelopeError::BuilderIndexMismatch { + committed_bid: execution_bid.builder_index, + envelope: envelope.builder_index, + }); + } + + // the block hash should match the block hash of the execution bid + if payload.block_hash != execution_bid.block_hash { + return Err(EnvelopeError::BlockHashMismatch { + committed_bid: execution_bid.block_hash, + envelope: payload.block_hash, + }); + } + + // Get the fork from the proposer cache so we can verify the signature. + // This is currently the most efficient way to implement envelope signature verification + // because the `fork` might depend on advancing the parent state. + let block_slot = envelope.slot; + let block_epoch = block_slot.epoch(T::EthSpec::slots_per_epoch()); + let proposer_shuffling_decision_block = + proto_block.proposer_shuffling_root_for_child_block(block_epoch, &chain.spec); + let mut opt_snapshot = None; + let envelope_ref = signed_envelope.as_ref(); + let proposer = chain.with_proposer_cache::<_, EnvelopeError>( + proposer_shuffling_decision_block, + block_epoch, + |proposers| proposers.get_slot::(block_slot), + || { + debug!( + %beacon_block_root, + block_hash = %envelope_ref.block_hash(), + "Proposer shuffling cache miss for envelope verification" + ); + // The proposer index was *not* cached and we must load the parent in order to + // determine the proposer index. + let snapshot = load_snapshot(envelope_ref, chain)?; + opt_snapshot = Some(Box::new(snapshot.clone())); + Ok((snapshot.state_root, snapshot.pre_state)) + }, + )?; + let fork = proposer.fork; + + // True builder index accounting for self-building. + let proposer_index = block.message().proposer_index(); + let builder_index = envelope.builder_index; + + let signature_is_valid = { + let pubkey_cache = chain.validator_pubkey_cache.read(); + let builder_pubkey = pubkey_cache + .get(builder_index as usize) + .ok_or_else(|| EnvelopeError::UnknownValidator { builder_index })?; + signed_envelope.verify_signature( + builder_pubkey, + &fork, + chain.genesis_validators_root, + &chain.spec, + ) + }; + + if !signature_is_valid { + return Err(EnvelopeError::BadSignature); + } + + Ok(Self { + signed_envelope, + block, + snapshot: opt_snapshot, + }) + } + + pub fn envelope_cloned(&self) -> Arc> { + self.signed_envelope.clone() + } +} + +pub trait IntoExecutionPendingEnvelope: Sized { + fn into_execution_pending_envelope( + self, + chain: &Arc>, + notify_execution_layer: NotifyExecutionLayer, + ) -> Result, EnvelopeError>; +} + +pub struct ExecutionPendingEnvelope { + pub signed_envelope: MaybeAvailableEnvelope, + pub import_data: EnvelopeImportData, + pub payload_verification_handle: PayloadVerificationHandle, +} + +impl IntoExecutionPendingEnvelope for GossipVerifiedEnvelope { + fn into_execution_pending_envelope( + self, + chain: &Arc>, + notify_execution_layer: NotifyExecutionLayer, + ) -> Result, EnvelopeError> { + let signed_envelope = self.signed_envelope; + let envelope = &signed_envelope.message; + let payload = &envelope.payload; + + // Verify the execution payload is valid + let payload_notifier = + PayloadNotifier::from_envelope(chain.clone(), envelope, notify_execution_layer)?; + let block_root = envelope.beacon_block_root; + let slot = self.block.slot(); + + let payload_verification_future = async move { + let chain = payload_notifier.chain.clone(); + // TODO:(gloas): timing metrics + if let Some(started_execution) = chain.slot_clock.now_duration() { + chain.block_times_cache.write().set_time_started_execution( + block_root, + slot, + started_execution, + ); + } + + let payload_verification_status = payload_notifier.notify_new_payload().await?; + Ok(PayloadVerificationOutcome { + payload_verification_status, + // This fork is after the merge so it'll never be the merge transition block + is_valid_merge_transition_block: false, + }) + }; + // Spawn the payload verification future as a new task, but don't wait for it to complete. + // The `payload_verification_future` will be awaited later to ensure verification completed + // successfully. + let payload_verification_handle = chain + .task_executor + .spawn_handle( + payload_verification_future, + "execution_payload_verification", + ) + .ok_or(BeaconChainError::RuntimeShutdown)?; + + let snapshot = if let Some(snapshot) = self.snapshot { + *snapshot + } else { + load_snapshot(signed_envelope.as_ref(), chain)? + }; + let mut state = snapshot.pre_state; + + // All the state modifications are done in envelope_processing + process_execution_payload_envelope( + &mut state, + Some(snapshot.state_root), + &signed_envelope, + // verify signature already done for GossipVerifiedEnvelope + VerifySignatures::False, + &chain.spec, + )?; + + Ok(ExecutionPendingEnvelope { + signed_envelope: MaybeAvailableEnvelope::AvailabilityPending { + block_hash: payload.block_hash, + envelope: signed_envelope, + }, + import_data: EnvelopeImportData { + block_root, + block: self.block, + post_state: Box::new(state), + }, + payload_verification_handle, + }) + } +} + +impl IntoExecutionPendingEnvelope + for Arc> +{ + fn into_execution_pending_envelope( + self, + chain: &Arc>, + notify_execution_layer: NotifyExecutionLayer, + ) -> Result, EnvelopeError> { + // TODO(EIP-7732): figure out how this should be refactored.. + GossipVerifiedEnvelope::new(self, chain)? + .into_execution_pending_envelope(chain, notify_execution_layer) + } +} \ No newline at end of file diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification_types.rs b/beacon_node/beacon_chain/src/payload_envelope_verification_types.rs new file mode 100644 index 0000000000..bd2eb5a34f --- /dev/null +++ b/beacon_node/beacon_chain/src/payload_envelope_verification_types.rs @@ -0,0 +1,31 @@ +use std::sync::Arc; +use types::{ + BeaconState, ChainSpec, DataColumnSidecarList, EthSpec, ExecutionBlockHash, Hash256, + SignedBeaconBlock, SignedExecutionPayloadEnvelope, +}; + +#[derive(PartialEq)] +pub struct EnvelopeImportData { + pub block_root: Hash256, + pub block: Arc>, + pub post_state: Box>, +} + +#[derive(Debug)] +#[allow(dead_code)] +pub struct AvailableEnvelope { + // TODO(EIP-7732): rename to execution_block_hash + block_hash: ExecutionBlockHash, + envelope: Arc>, + columns: DataColumnSidecarList, + /// Timestamp at which this block first became available (UNIX timestamp, time since 1970). + columns_available_timestamp: Option, + pub spec: Arc, +} +pub enum MaybeAvailableEnvelope { + Available(AvailableEnvelope), + AvailabilityPending { + block_hash: ExecutionBlockHash, + envelope: Arc>, + }, +} \ No newline at end of file From 8204241b456f739c01c14596d35cdbeb10b9ad7f Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Tue, 10 Feb 2026 19:57:53 -0800 Subject: [PATCH 02/19] Progress --- beacon_node/beacon_chain/src/beacon_chain.rs | 1 - .../beacon_chain/src/block_verification.rs | 3 +- .../beacon_chain/src/execution_payload.rs | 31 +-- beacon_node/beacon_chain/src/lib.rs | 1 - .../execution_pending_envelope.rs | 26 ++ .../gossip_verified_envelope.rs} | 210 +--------------- .../src/payload_envelope_verification/mod.rs | 229 ++++++++++++++++++ .../payload_notifier.rs | 77 ++++++ .../payload_envelope_verification_types.rs | 31 --- .../src/engine_api/new_payload_request.rs | 3 +- .../src/envelope_processing.rs | 101 ++++++++ 11 files changed, 464 insertions(+), 249 deletions(-) create mode 100644 beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs rename beacon_node/beacon_chain/src/{payload_envelope_verification.rs => payload_envelope_verification/gossip_verified_envelope.rs} (55%) create mode 100644 beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs create mode 100644 beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs delete mode 100644 beacon_node/beacon_chain/src/payload_envelope_verification_types.rs diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index a59508a37a..801ef7696f 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1147,7 +1147,6 @@ impl BeaconChain { } } - /// Returns the full block at the given root, if it's available in the database. /// /// Should always return a full block for pre-merge and post-gloas blocks. diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 9d1b8e0d3d..c11f26aa00 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -718,7 +718,8 @@ pub struct SignatureVerifiedBlock { } /// Used to await the result of executing payload with an EE. -pub type PayloadVerificationHandle = JoinHandle>>; +pub type PayloadVerificationHandle = + JoinHandle>>; /// A wrapper around a `SignedBeaconBlock` that indicates that this block is fully verified and /// ready to import into the `BeaconChain`. The validation includes: diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index 370f044c7a..b8fbbc3c32 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -113,7 +113,7 @@ impl PayloadNotifier { if let Some(precomputed_status) = self.payload_verification_status { Ok(precomputed_status) } else { - notify_new_payload(&self.chain, self.block.message()).await + notify_new_payload(&self.chain, self.block.message().tree_hash_root(), self.block.message().try_into()?).await } } } @@ -127,17 +127,18 @@ impl PayloadNotifier { /// contains a few extra checks by running `partially_verify_execution_payload` first: /// /// https://github.com/ethereum/consensus-specs/blob/v1.1.9/specs/bellatrix/beacon-chain.md#notify_new_payload -async fn notify_new_payload( +pub async fn notify_new_payload( chain: &Arc>, - block: BeaconBlockRef<'_, T::EthSpec>, + beacon_block_root: Hash256, + new_payload_request: NewPayloadRequest<'_, T::EthSpec>, ) -> Result { let execution_layer = chain .execution_layer .as_ref() .ok_or(ExecutionPayloadError::NoExecutionConnection)?; - let execution_block_hash = block.execution_payload()?.block_hash(); - let new_payload_response = execution_layer.notify_new_payload(block.try_into()?).await; + let execution_block_hash = new_payload_request.execution_payload_ref().block_hash(); + let new_payload_response = execution_layer.notify_new_payload(new_payload_request.clone()).await; match new_payload_response { Ok(status) => match status { @@ -153,10 +154,10 @@ async fn notify_new_payload( ?validation_error, ?latest_valid_hash, ?execution_block_hash, - root = ?block.tree_hash_root(), - graffiti = block.body().graffiti().as_utf8_lossy(), - proposer_index = block.proposer_index(), - slot = %block.slot(), + // root = ?block.tree_hash_root(), + // graffiti = block.body().graffiti().as_utf8_lossy(), + // proposer_index = block.proposer_index(), + // slot = %block.slot(), method = "new_payload", "Invalid execution payload" ); @@ -179,11 +180,11 @@ async fn notify_new_payload( { // This block has not yet been applied to fork choice, so the latest block that was // imported to fork choice was the parent. - let latest_root = block.parent_root(); + let latest_root = new_payload_request.parent_beacon_block_root()?; chain .process_invalid_execution_payload(&InvalidationOperation::InvalidateMany { - head_block_root: latest_root, + head_block_root: *latest_root, always_invalidate_head: false, latest_valid_ancestor: latest_valid_hash, }) @@ -198,10 +199,10 @@ async fn notify_new_payload( warn!( ?validation_error, ?execution_block_hash, - root = ?block.tree_hash_root(), - graffiti = block.body().graffiti().as_utf8_lossy(), - proposer_index = block.proposer_index(), - slot = %block.slot(), + // root = ?block.tree_hash_root(), + // graffiti = block.body().graffiti().as_utf8_lossy(), + // proposer_index = block.proposer_index(), + // slot = %block.slot(), method = "new_payload", "Invalid execution payload block hash" ); diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index 7aa64deb1c..4120ed86fc 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -43,7 +43,6 @@ pub mod observed_data_sidecars; pub mod observed_operations; mod observed_slashable; pub mod payload_envelope_verification; -pub mod payload_envelope_verification_types; pub mod persisted_beacon_chain; pub mod persisted_custody; mod persisted_fork_choice; diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs new file mode 100644 index 0000000000..956e6ffb8f --- /dev/null +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs @@ -0,0 +1,26 @@ +use task_executor::JoinHandle; +use types::{EthSpec, FullPayload}; + +use crate::{BeaconChainTypes, PayloadVerificationOutcome, payload_envelope_verification::PayloadEnvelopeImportData}; + + +/// Used to await the result of executing payload with an EE. +pub type PayloadVerificationHandle = + JoinHandle>>>; + +/// A wrapper around a `SignedBeaconBlock` that indicates that this block is fully verified and +/// ready to import into the `BeaconChain`. The validation includes: +/// +/// - Parent is known +/// - Signatures +/// - State root check +/// - Block processing +/// +/// Note: a `ExecutionPendingEnvelope` is not _forever_ valid to be imported, it may later become invalid +/// due to finality or some other event. A `ExecutionPendingEnvelope` should be imported into the +/// `BeaconChain` immediately after it is instantiated. +pub struct ExecutionPendingEnvelope { + pub block: MaybeAvailableBlock, + pub import_data: PayloadEnvelopeImportData, + pub payload_verification_handle: PayloadVerificationHandle, +} \ No newline at end of file diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs similarity index 55% rename from beacon_node/beacon_chain/src/payload_envelope_verification.rs rename to beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs index b22acd69fb..2bd5a6c3d7 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs @@ -1,175 +1,14 @@ -//! The incremental processing steps (e.g., signatures verified but not the state transition) is -//! represented as a sequence of wrapper-types around the block. There is a linear progression of -//! types, starting at a `SignedBeaconBlock` and finishing with a `Fully VerifiedBlock` (see -//! diagram below). -//! -//! ```ignore -//! START -//! | -//! ▼ -//! SignedExecutionPayloadEnvelope -//! | -//! |--------------- -//! | | -//! | ▼ -//! | GossipVerifiedEnvelope -//! | | -//! |--------------- -//! | -//! ▼ -//! ExecutionPendingEnvelope -//! | -//! await -//! | -//! ▼ -//! END -//! -//! ``` - -use crate::NotifyExecutionLayer; -use crate::block_verification::{PayloadVerificationHandle, PayloadVerificationOutcome}; -use crate::payload_envelope_verification_types::{EnvelopeImportData, MaybeAvailableEnvelope}; -use crate::execution_payload::PayloadNotifier; -use crate::{BeaconChain, BeaconChainError, BeaconChainTypes}; -use educe::Educe; -use slot_clock::SlotClock; -use state_processing::envelope_processing::{EnvelopeProcessingError, process_execution_payload_envelope}; -use state_processing::{BlockProcessingError, VerifySignatures}; use std::sync::Arc; -use tracing::{debug, instrument}; -use types::{ - BeaconState, BeaconStateError, EthSpec, ExecutionBlockHash, Hash256, SignedBeaconBlock, - SignedExecutionPayloadEnvelope, Slot, + +use educe::Educe; +use state_processing::{VerifySignatures, envelope_processing::process_execution_payload_envelope}; +use tracing::debug; +use types::{EthSpec, SignedBeaconBlock, SignedExecutionPayloadEnvelope}; + +use crate::{ + BeaconChain, BeaconChainError, BeaconChainTypes, NotifyExecutionLayer, PayloadVerificationOutcome, payload_envelope_verification::{EnvelopeError, EnvelopeImportData, EnvelopeProcessingSnapshot, ExecutionPendingEnvelope, IntoExecutionPendingEnvelope, MaybeAvailableEnvelope, load_snapshot, payload_notifier::PayloadNotifier} }; -#[derive(Debug, Clone)] -pub enum EnvelopeError { - /// The envelope's block root is unknown. - BlockRootUnknown { - block_root: Hash256, - }, - /// The signature is invalid. - BadSignature, - /// The builder index doesn't match the committed bid - BuilderIndexMismatch { - committed_bid: u64, - envelope: u64, - }, - // The envelope slot doesn't match the block - SlotMismatch { - block: Slot, - envelope: Slot, - }, - // The validator index is unknown - UnknownValidator { - builder_index: u64, - }, - // The block hash doesn't match the committed bid - BlockHashMismatch { - committed_bid: ExecutionBlockHash, - envelope: ExecutionBlockHash, - }, - // Some Beacon Chain Error - BeaconChainError(Arc), - // Some Beacon State error - BeaconStateError(BeaconStateError), - // Some BlockProcessingError (for electra operations) - BlockProcessingError(BlockProcessingError), - // Some EnvelopeProcessingError - EnvelopeProcessingError(EnvelopeProcessingError), -} - -impl From for EnvelopeError { - fn from(e: BeaconChainError) -> Self { - EnvelopeError::BeaconChainError(Arc::new(e)) - } -} - -impl From for EnvelopeError { - fn from(e: BeaconStateError) -> Self { - EnvelopeError::BeaconStateError(e) - } -} - -/// Pull errors up from EnvelopeProcessingError to EnvelopeError -impl From for EnvelopeError { - fn from(e: EnvelopeProcessingError) -> Self { - match e { - EnvelopeProcessingError::BadSignature => EnvelopeError::BadSignature, - EnvelopeProcessingError::BeaconStateError(e) => EnvelopeError::BeaconStateError(e), - EnvelopeProcessingError::BlockHashMismatch { - committed_bid, - envelope, - } => EnvelopeError::BlockHashMismatch { - committed_bid, - envelope, - }, - EnvelopeProcessingError::BlockProcessingError(e) => { - EnvelopeError::BlockProcessingError(e) - } - e => EnvelopeError::EnvelopeProcessingError(e), - } - } -} - -/// This snapshot is to be used for verifying a envelope of the block. -#[derive(Debug, Clone)] -pub struct EnvelopeProcessingSnapshot { - /// This state is equivalent to the `self.beacon_block.state_root()` before applying the envelope. - pub pre_state: BeaconState, - pub state_root: Hash256, - pub beacon_block_root: Hash256, -} - -#[allow(clippy::type_complexity)] -#[instrument(skip_all, level = "debug", fields(beacon_block_root = %envelope.beacon_block_root()))] -fn load_snapshot( - envelope: &SignedExecutionPayloadEnvelope, - chain: &BeaconChain, -) -> Result, EnvelopeError> { - // Reject any block if its block is not known to fork choice. - // - // A block that is not in fork choice is either: - // - // - Not yet imported: we should reject this block because we should only import a child - // envelope after its parent has been fully imported. - // - Pre-finalized: if the block is _prior_ to finalization, we should ignore the envelope - // because it will revert finalization. Note that the finalized block is stored in fork - // choice, so we will not reject any child of the finalized block (this is relevant during - // genesis). - - let fork_choice_read_lock = chain.canonical_head.fork_choice_read_lock(); - let beacon_block_root = envelope.beacon_block_root(); - let Some(proto_beacon_block) = fork_choice_read_lock.get_block(&beacon_block_root) else { - return Err(EnvelopeError::BlockRootUnknown { - block_root: beacon_block_root, - }); - }; - drop(fork_choice_read_lock); - - // TODO(EIP-7732): add metrics here - - let block_state_root = proto_beacon_block.state_root; - // We can use `get_hot_state` here rather than `get_advanced_hot_state` because the envelope - // must be from the same slot as its block (so no advance is required). - let cache_state = true; - let state = chain - .store - .get_hot_state(&block_state_root, cache_state) - .map_err(|e| EnvelopeError::BeaconChainError(Arc::new(e.into())))? - .ok_or_else(|| { - BeaconChainError::DBInconsistent(format!( - "Missing state for envelope block {block_state_root:?}", - )) - })?; - - Ok(EnvelopeProcessingSnapshot { - pre_state: state, - state_root: block_state_root, - beacon_block_root, - }) -} - /// A wrapper around a `SignedExecutionPayloadEnvelope` that indicates it has been approved for re-gossiping on /// the p2p network. #[derive(Educe)] @@ -313,20 +152,6 @@ impl GossipVerifiedEnvelope { } } -pub trait IntoExecutionPendingEnvelope: Sized { - fn into_execution_pending_envelope( - self, - chain: &Arc>, - notify_execution_layer: NotifyExecutionLayer, - ) -> Result, EnvelopeError>; -} - -pub struct ExecutionPendingEnvelope { - pub signed_envelope: MaybeAvailableEnvelope, - pub import_data: EnvelopeImportData, - pub payload_verification_handle: PayloadVerificationHandle, -} - impl IntoExecutionPendingEnvelope for GossipVerifiedEnvelope { fn into_execution_pending_envelope( self, @@ -336,10 +161,13 @@ impl IntoExecutionPendingEnvelope for GossipVerifiedEnve let signed_envelope = self.signed_envelope; let envelope = &signed_envelope.message; let payload = &envelope.payload; + + // TODO(gloas) unwrap + let bid = chain.get_full_block(&envelope.beacon_block_root).unwrap().unwrap().message().body().signed_execution_payload_bid().unwrap().message; // Verify the execution payload is valid let payload_notifier = - PayloadNotifier::from_envelope(chain.clone(), envelope, notify_execution_layer)?; + PayloadNotifier::new(chain.clone(), envelope, notify_execution_layer)?; let block_root = envelope.beacon_block_root; let slot = self.block.slot(); @@ -403,17 +231,3 @@ impl IntoExecutionPendingEnvelope for GossipVerifiedEnve }) } } - -impl IntoExecutionPendingEnvelope - for Arc> -{ - fn into_execution_pending_envelope( - self, - chain: &Arc>, - notify_execution_layer: NotifyExecutionLayer, - ) -> Result, EnvelopeError> { - // TODO(EIP-7732): figure out how this should be refactored.. - GossipVerifiedEnvelope::new(self, chain)? - .into_execution_pending_envelope(chain, notify_execution_layer) - } -} \ No newline at end of file diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs new file mode 100644 index 0000000000..4a739e74c5 --- /dev/null +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs @@ -0,0 +1,229 @@ +//! The incremental processing steps (e.g., signatures verified but not the state transition) is +//! represented as a sequence of wrapper-types around the block. There is a linear progression of +//! types, starting at a `SignedBeaconBlock` and finishing with a `Fully VerifiedBlock` (see +//! diagram below). +//! +//! ```ignore +//! START +//! | +//! ▼ +//! SignedExecutionPayloadEnvelope +//! | +//! |--------------- +//! | | +//! | ▼ +//! | GossipVerifiedEnvelope +//! | | +//! |--------------- +//! | +//! ▼ +//! ExecutionPendingEnvelope +//! | +//! await +//! | +//! ▼ +//! END +//! +//! ``` + +use std::sync::Arc; + +use state_processing::{BlockProcessingError, ConsensusContext, envelope_processing::EnvelopeProcessingError}; +use tracing::instrument; +use types::{BeaconState, BeaconStateError, ChainSpec, DataColumnSidecarList, EthSpec, ExecutionBlockHash, Hash256, SignedBeaconBlock, SignedExecutionPayloadEnvelope, Slot}; + +use crate::{BeaconChain, BeaconChainError, BeaconChainTypes, NotifyExecutionLayer, block_verification::PayloadVerificationHandle, payload_envelope_verification::gossip_verified_envelope::GossipVerifiedEnvelope}; + +pub mod execution_pending_envelope; +pub mod gossip_verified_envelope; +mod payload_notifier; + +pub trait IntoExecutionPendingEnvelope: Sized { + fn into_execution_pending_envelope( + self, + chain: &Arc>, + notify_execution_layer: NotifyExecutionLayer, + ) -> Result, EnvelopeError>; +} + +pub struct ExecutionPendingEnvelope { + pub signed_envelope: MaybeAvailableEnvelope, + pub import_data: EnvelopeImportData, + pub payload_verification_handle: PayloadVerificationHandle, +} + +#[derive(PartialEq)] +pub struct EnvelopeImportData { + pub block_root: Hash256, + pub block: Arc>, + pub post_state: Box>, +} + +#[derive(Debug)] +#[allow(dead_code)] +pub struct AvailableEnvelope { + // TODO(EIP-7732): rename to execution_block_hash + block_hash: ExecutionBlockHash, + envelope: Arc>, + columns: DataColumnSidecarList, + /// Timestamp at which this block first became available (UNIX timestamp, time since 1970). + columns_available_timestamp: Option, + pub spec: Arc, +} + +pub enum MaybeAvailableEnvelope { + Available(AvailableEnvelope), + AvailabilityPending { + block_hash: ExecutionBlockHash, + envelope: Arc>, + }, +} + +/// This snapshot is to be used for verifying a envelope of the block. +#[derive(Debug, Clone)] +pub struct EnvelopeProcessingSnapshot { + /// This state is equivalent to the `self.beacon_block.state_root()` before applying the envelope. + pub pre_state: BeaconState, + pub state_root: Hash256, + pub beacon_block_root: Hash256, +} + +#[derive(Debug, Clone)] +pub enum EnvelopeError { + /// The envelope's block root is unknown. + BlockRootUnknown { + block_root: Hash256, + }, + /// The signature is invalid. + BadSignature, + /// The builder index doesn't match the committed bid + BuilderIndexMismatch { + committed_bid: u64, + envelope: u64, + }, + // The envelope slot doesn't match the block + SlotMismatch { + block: Slot, + envelope: Slot, + }, + // The validator index is unknown + UnknownValidator { + builder_index: u64, + }, + // The block hash doesn't match the committed bid + BlockHashMismatch { + committed_bid: ExecutionBlockHash, + envelope: ExecutionBlockHash, + }, + // Some Beacon Chain Error + BeaconChainError(Arc), + // Some Beacon State error + BeaconStateError(BeaconStateError), + // Some BlockProcessingError (for electra operations) + BlockProcessingError(BlockProcessingError), + // Some EnvelopeProcessingError + EnvelopeProcessingError(EnvelopeProcessingError), +} + +impl From for EnvelopeError { + fn from(e: BeaconChainError) -> Self { + EnvelopeError::BeaconChainError(Arc::new(e)) + } +} + +impl From for EnvelopeError { + fn from(e: BeaconStateError) -> Self { + EnvelopeError::BeaconStateError(e) + } +} + +/// Pull errors up from EnvelopeProcessingError to EnvelopeError +impl From for EnvelopeError { + fn from(e: EnvelopeProcessingError) -> Self { + match e { + EnvelopeProcessingError::BadSignature => EnvelopeError::BadSignature, + EnvelopeProcessingError::BeaconStateError(e) => EnvelopeError::BeaconStateError(e), + EnvelopeProcessingError::BlockHashMismatch { + committed_bid, + envelope, + } => EnvelopeError::BlockHashMismatch { + committed_bid, + envelope, + }, + EnvelopeProcessingError::BlockProcessingError(e) => { + EnvelopeError::BlockProcessingError(e) + } + e => EnvelopeError::EnvelopeProcessingError(e), + } + } +} + +#[allow(clippy::type_complexity)] +#[instrument(skip_all, level = "debug", fields(beacon_block_root = %envelope.beacon_block_root()))] +pub(crate) fn load_snapshot( + envelope: &SignedExecutionPayloadEnvelope, + chain: &BeaconChain, +) -> Result, EnvelopeError> { + // Reject any block if its block is not known to fork choice. + // + // A block that is not in fork choice is either: + // + // - Not yet imported: we should reject this block because we should only import a child + // envelope after its parent has been fully imported. + // - Pre-finalized: if the block is _prior_ to finalization, we should ignore the envelope + // because it will revert finalization. Note that the finalized block is stored in fork + // choice, so we will not reject any child of the finalized block (this is relevant during + // genesis). + + let fork_choice_read_lock = chain.canonical_head.fork_choice_read_lock(); + let beacon_block_root = envelope.beacon_block_root(); + let Some(proto_beacon_block) = fork_choice_read_lock.get_block(&beacon_block_root) else { + return Err(EnvelopeError::BlockRootUnknown { + block_root: beacon_block_root, + }); + }; + drop(fork_choice_read_lock); + + // TODO(EIP-7732): add metrics here + + let block_state_root = proto_beacon_block.state_root; + // We can use `get_hot_state` here rather than `get_advanced_hot_state` because the envelope + // must be from the same slot as its block (so no advance is required). + let cache_state = true; + let state = chain + .store + .get_hot_state(&block_state_root, cache_state) + .map_err(|e| EnvelopeError::BeaconChainError(Arc::new(e.into())))? + .ok_or_else(|| { + BeaconChainError::DBInconsistent(format!( + "Missing state for envelope block {block_state_root:?}", + )) + })?; + + Ok(EnvelopeProcessingSnapshot { + pre_state: state, + state_root: block_state_root, + beacon_block_root, + }) +} + +impl IntoExecutionPendingEnvelope + for Arc> +{ + fn into_execution_pending_envelope( + self, + chain: &Arc>, + notify_execution_layer: NotifyExecutionLayer, + ) -> Result, EnvelopeError> { + // TODO(EIP-7732): figure out how this should be refactored.. + GossipVerifiedEnvelope::new(self, chain)? + .into_execution_pending_envelope(chain, notify_execution_layer) + } +} + +#[derive(Clone, Debug, PartialEq)] +pub struct PayloadEnvelopeImportData { + pub block_root: Hash256, + pub state: BeaconState, + pub consensus_context: ConsensusContext, +} diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs new file mode 100644 index 0000000000..15fec7e21d --- /dev/null +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs @@ -0,0 +1,77 @@ +use std::sync::Arc; + +use execution_layer::NewPayloadRequest; +use fork_choice::PayloadVerificationStatus; +use state_processing::{envelope_processing::partially_verify_payload_envelope, per_block_processing::is_execution_enabled}; +use tracing::warn; +use types::{BeaconState, ExecutionPayloadBid, Hash256, SignedBeaconBlock, SignedExecutionPayloadEnvelope}; + +use crate::{BeaconChain, BeaconChainTypes, BlockError, ExecutionPayloadError, NotifyExecutionLayer, execution_payload::notify_new_payload}; + + +/// Used to await the result of executing payload with a remote EE. +pub struct PayloadNotifier { + pub chain: Arc>, + pub envelope: Arc>, + payload_verification_status: Option, +} + +impl PayloadNotifier { + pub fn new( + chain: Arc>, + bid: &ExecutionPayloadBid, + envelope: Arc>, + state: &BeaconState, + notify_execution_layer: NotifyExecutionLayer, + ) -> Result { + let payload_verification_status = { + // Perform the initial stages of payload verification. + // + // We will duplicate these checks again during `per_block_processing`, however these + // checks are cheap and doing them here ensures we have verified them before marking + // the block as optimistically imported. This is particularly relevant in the case + // where we do not send the block to the EL at all. + let payload_message = &envelope.message; + partially_verify_payload_envelope( + state, + &envelope, + &chain.spec, + ).unwrap(); // TODO(gloas) unwrap + + match notify_execution_layer { + NotifyExecutionLayer::No if chain.config.optimistic_finalized_sync => { + // Create a NewPayloadRequest (no clones required) and check optimistic sync verifications + let new_payload_request: NewPayloadRequest = + payload_message.try_into()?; + if let Err(e) = new_payload_request.perform_optimistic_sync_verifications() { + warn!( + block_number = ?payload_message.payload.block_number, + info = "you can silence this warning with --disable-optimistic-finalized-sync", + error = ?e, + "Falling back to slow block hash verification" + ); + None + } else { + Some(PayloadVerificationStatus::Optimistic) + } + } + _ => None, + } + }; + + Ok(Self { + chain, + envelope, + payload_verification_status, + }) + } + + pub async fn notify_new_payload(self) -> Result { + if let Some(precomputed_status) = self.payload_verification_status { + Ok(precomputed_status) + } else { + // tODO(gloas) fix zero + notify_new_payload(&self.chain, Hash256::ZERO, self.envelope.message.try_into()?).await + } + } +} \ No newline at end of file diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification_types.rs b/beacon_node/beacon_chain/src/payload_envelope_verification_types.rs deleted file mode 100644 index bd2eb5a34f..0000000000 --- a/beacon_node/beacon_chain/src/payload_envelope_verification_types.rs +++ /dev/null @@ -1,31 +0,0 @@ -use std::sync::Arc; -use types::{ - BeaconState, ChainSpec, DataColumnSidecarList, EthSpec, ExecutionBlockHash, Hash256, - SignedBeaconBlock, SignedExecutionPayloadEnvelope, -}; - -#[derive(PartialEq)] -pub struct EnvelopeImportData { - pub block_root: Hash256, - pub block: Arc>, - pub post_state: Box>, -} - -#[derive(Debug)] -#[allow(dead_code)] -pub struct AvailableEnvelope { - // TODO(EIP-7732): rename to execution_block_hash - block_hash: ExecutionBlockHash, - envelope: Arc>, - columns: DataColumnSidecarList, - /// Timestamp at which this block first became available (UNIX timestamp, time since 1970). - columns_available_timestamp: Option, - pub spec: Arc, -} -pub enum MaybeAvailableEnvelope { - Available(AvailableEnvelope), - AvailabilityPending { - block_hash: ExecutionBlockHash, - envelope: Arc>, - }, -} \ No newline at end of file diff --git a/beacon_node/execution_layer/src/engine_api/new_payload_request.rs b/beacon_node/execution_layer/src/engine_api/new_payload_request.rs index ba94296b85..b88dd7ca3a 100644 --- a/beacon_node/execution_layer/src/engine_api/new_payload_request.rs +++ b/beacon_node/execution_layer/src/engine_api/new_payload_request.rs @@ -4,8 +4,7 @@ use crate::versioned_hashes::verify_versioned_hashes; use state_processing::per_block_processing::deneb::kzg_commitment_to_versioned_hash; use superstruct::superstruct; use types::{ - BeaconBlockRef, BeaconStateError, EthSpec, ExecutionBlockHash, ExecutionPayload, - ExecutionPayloadRef, Hash256, VersionedHash, + BeaconBlockRef, BeaconStateError, EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadEnvelope, ExecutionPayloadRef, Hash256, VersionedHash }; use types::{ ExecutionPayloadBellatrix, ExecutionPayloadCapella, ExecutionPayloadDeneb, diff --git a/consensus/state_processing/src/envelope_processing.rs b/consensus/state_processing/src/envelope_processing.rs index d46728dbbc..2f511846a5 100644 --- a/consensus/state_processing/src/envelope_processing.rs +++ b/consensus/state_processing/src/envelope_processing.rs @@ -276,3 +276,104 @@ pub fn process_execution_payload_envelope( Ok(()) } + + +/// Performs *partial* verification of the `payload envelope`. +pub fn partially_verify_payload_envelope( + state: &BeaconState, + signed_envelope: &SignedExecutionPayloadEnvelope, + spec: &ChainSpec, +) -> Result<(), EnvelopeProcessingError> { + let envelope = &signed_envelope.message; + let payload = &signed_envelope.message.payload; + + // Verify consistency with the beacon block + let latest_block_header_root = state.latest_block_header().tree_hash_root(); + envelope_verify!( + envelope.beacon_block_root == latest_block_header_root, + EnvelopeProcessingError::LatestBlockHeaderMismatch { + envelope_root: envelope.beacon_block_root, + block_header_root: latest_block_header_root, + } + ); + envelope_verify!( + envelope.slot == state.slot(), + EnvelopeProcessingError::SlotMismatch { + envelope_slot: envelope.slot, + parent_state_slot: state.slot(), + } + ); + + // Verify consistency with the committed bid + let committed_bid = state.latest_execution_payload_bid()?; + envelope_verify!( + envelope.builder_index == committed_bid.builder_index, + EnvelopeProcessingError::BuilderIndexMismatch { + committed_bid: committed_bid.builder_index, + envelope: envelope.builder_index, + } + ); + envelope_verify!( + committed_bid.prev_randao == payload.prev_randao, + EnvelopeProcessingError::PrevRandaoMismatch { + committed_bid: committed_bid.prev_randao, + envelope: payload.prev_randao, + } + ); + + // Verify consistency with expected withdrawals + // NOTE: we don't bother hashing here except in case of error, because we can just compare for + // equality directly. This equality check could be more straight-forward if the types were + // changed to match (currently we are comparing VariableList to List). This could happen + // coincidentally when we adopt ProgressiveList. + envelope_verify!( + payload.withdrawals.len() == state.payload_expected_withdrawals()?.len() + && payload + .withdrawals + .iter() + .eq(state.payload_expected_withdrawals()?.iter()), + EnvelopeProcessingError::WithdrawalsRootMismatch { + state: state.payload_expected_withdrawals()?.tree_hash_root(), + payload: payload.withdrawals.tree_hash_root(), + } + ); + + // Verify the gas limit + envelope_verify!( + committed_bid.gas_limit == payload.gas_limit, + EnvelopeProcessingError::GasLimitMismatch { + committed_bid: committed_bid.gas_limit, + envelope: payload.gas_limit, + } + ); + + // Verify the block hash + envelope_verify!( + committed_bid.block_hash == payload.block_hash, + EnvelopeProcessingError::BlockHashMismatch { + committed_bid: committed_bid.block_hash, + envelope: payload.block_hash, + } + ); + + // Verify consistency of the parent hash with respect to the previous execution payload + envelope_verify!( + payload.parent_hash == *state.latest_block_hash()?, + EnvelopeProcessingError::ParentHashMismatch { + state: *state.latest_block_hash()?, + envelope: payload.parent_hash, + } + ); + + // Verify timestamp + let state_timestamp = compute_timestamp_at_slot(state, state.slot(), spec)?; + envelope_verify!( + payload.timestamp == state_timestamp, + EnvelopeProcessingError::TimestampMismatch { + state: state_timestamp, + envelope: payload.timestamp, + } + ); + + Ok(()) +} From 22f3fd4ccf6571644e6dee9478ab68fc1d17596f Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Tue, 10 Feb 2026 22:40:26 -0800 Subject: [PATCH 03/19] Continue --- .../beacon_chain/src/execution_payload.rs | 11 +++- .../execution_pending_envelope.rs | 12 ++-- .../gossip_verified_envelope.rs | 65 +++++++++++++------ .../src/payload_envelope_verification/mod.rs | 25 ++++++- .../payload_notifier.rs | 32 +++++---- .../signature_verified_envelope.rs | 40 ++++++++++++ .../src/engine_api/new_payload_request.rs | 3 +- .../src/envelope_processing.rs | 3 +- 8 files changed, 146 insertions(+), 45 deletions(-) create mode 100644 beacon_node/beacon_chain/src/payload_envelope_verification/signature_verified_envelope.rs diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index b8fbbc3c32..631a0180dd 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -113,7 +113,12 @@ impl PayloadNotifier { if let Some(precomputed_status) = self.payload_verification_status { Ok(precomputed_status) } else { - notify_new_payload(&self.chain, self.block.message().tree_hash_root(), self.block.message().try_into()?).await + notify_new_payload( + &self.chain, + self.block.message().tree_hash_root(), + self.block.message().try_into()?, + ) + .await } } } @@ -138,7 +143,9 @@ pub async fn notify_new_payload( .ok_or(ExecutionPayloadError::NoExecutionConnection)?; let execution_block_hash = new_payload_request.execution_payload_ref().block_hash(); - let new_payload_response = execution_layer.notify_new_payload(new_payload_request.clone()).await; + let new_payload_response = execution_layer + .notify_new_payload(new_payload_request.clone()) + .await; match new_payload_response { Ok(status) => match status { diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs index 956e6ffb8f..7dffd1c09c 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs @@ -1,8 +1,10 @@ use task_executor::JoinHandle; use types::{EthSpec, FullPayload}; -use crate::{BeaconChainTypes, PayloadVerificationOutcome, payload_envelope_verification::PayloadEnvelopeImportData}; - +use crate::{ + BeaconChainTypes, PayloadVerificationOutcome, + payload_envelope_verification::{MaybeAvailableEnvelope, PayloadEnvelopeImportData}, +}; /// Used to await the result of executing payload with an EE. pub type PayloadVerificationHandle = @@ -20,7 +22,7 @@ pub type PayloadVerificationHandle = /// due to finality or some other event. A `ExecutionPendingEnvelope` should be imported into the /// `BeaconChain` immediately after it is instantiated. pub struct ExecutionPendingEnvelope { - pub block: MaybeAvailableBlock, + pub block: MaybeAvailableEnvelope, pub import_data: PayloadEnvelopeImportData, - pub payload_verification_handle: PayloadVerificationHandle, -} \ No newline at end of file + pub payload_verification_handle: PayloadVerificationHandle, +} diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs index 2bd5a6c3d7..580c6e9234 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs @@ -1,12 +1,19 @@ use std::sync::Arc; use educe::Educe; +use slot_clock::SlotClock; use state_processing::{VerifySignatures, envelope_processing::process_execution_payload_envelope}; use tracing::debug; -use types::{EthSpec, SignedBeaconBlock, SignedExecutionPayloadEnvelope}; +use types::{EthSpec, SignedBeaconBlock, SignedExecutionPayloadEnvelope, consts::gloas::BUILDER_INDEX_SELF_BUILD}; use crate::{ - BeaconChain, BeaconChainError, BeaconChainTypes, NotifyExecutionLayer, PayloadVerificationOutcome, payload_envelope_verification::{EnvelopeError, EnvelopeImportData, EnvelopeProcessingSnapshot, ExecutionPendingEnvelope, IntoExecutionPendingEnvelope, MaybeAvailableEnvelope, load_snapshot, payload_notifier::PayloadNotifier} + BeaconChain, BeaconChainError, BeaconChainTypes, NotifyExecutionLayer, + PayloadVerificationOutcome, + payload_envelope_verification::{ + EnvelopeError, EnvelopeImportData, EnvelopeProcessingSnapshot, ExecutionPendingEnvelope, + IntoExecutionPendingEnvelope, MaybeAvailableEnvelope, load_snapshot, + payload_notifier::PayloadNotifier, + }, }; /// A wrapper around a `SignedExecutionPayloadEnvelope` that indicates it has been approved for re-gossiping on @@ -29,7 +36,7 @@ impl GossipVerifiedEnvelope { let beacon_block_root = envelope.beacon_block_root; // Check that we've seen the beacon block for this envelope and that it passes validation. - // TODO(EIP-7732): We need a block status table in order to differentiate between: + // TODO(EIP-7732): We might need some type of status table in order to differentiate between: // // 1. Blocks we haven't seen (IGNORE), and // 2. Blocks we've seen that are invalid (REJECT). @@ -41,14 +48,16 @@ impl GossipVerifiedEnvelope { block_root: beacon_block_root, }); }; + + let latest_finalized_slot = fork_choice_read_lock + .finalized_checkpoint() + .epoch + .start_slot(T::EthSpec::slots_per_epoch()); + drop(fork_choice_read_lock); // TODO(EIP-7732): check that we haven't seen another valid `SignedExecutionPayloadEnvelope` // for this block root from this builder - envelope status table check - - // TODO(EIP-7732): this could be obtained from the ProtoBlock instead of the DB - // but this means the ProtoBlock needs to include something like the ExecutionBid - // will need to answer this question later. let block = chain .get_full_block(&beacon_block_root)? .ok_or_else(|| { @@ -61,11 +70,14 @@ impl GossipVerifiedEnvelope { .signed_execution_payload_bid()? .message; - // TODO(EIP-7732): Gossip rules for the beacon block contain the following: - // https://github.com/ethereum/consensus-specs/blob/master/specs/phase0/p2p-interface.md#beacon_block - // [IGNORE] The block is not from a future slot (with a MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance) - // [IGNORE] The block is from a slot greater than the latest finalized slot - // should these kinds of checks be included for envelopes as well? + // check that the envelopes slot isnt from a slot prior + // to the latest finalized slot. + if envelope.slot < latest_finalized_slot { + return Err(EnvelopeError::PriorToFinalization { + payload_slot: envelope.slot, + latest_finalized_slot, + }); + } // check that the slot of the envelope matches the slot of the parent block if envelope.slot != block.slot() { @@ -119,17 +131,22 @@ impl GossipVerifiedEnvelope { )?; let fork = proposer.fork; - // True builder index accounting for self-building. - let proposer_index = block.message().proposer_index(); let builder_index = envelope.builder_index; + let index = if builder_index == BUILDER_INDEX_SELF_BUILD { + block.message().proposer_index() + } else { + builder_index + }; let signature_is_valid = { + // TODO(gloas) the builder pubkey wont be in the validator pubkey cache + // this will currently only work for local block building. let pubkey_cache = chain.validator_pubkey_cache.read(); - let builder_pubkey = pubkey_cache - .get(builder_index as usize) - .ok_or_else(|| EnvelopeError::UnknownValidator { builder_index })?; + let pubkey = pubkey_cache + .get(index as usize) + .ok_or_else(|| EnvelopeError::UnknownValidator { builder_index: index })?; signed_envelope.verify_signature( - builder_pubkey, + pubkey, &fork, chain.genesis_validators_root, &chain.spec, @@ -161,9 +178,17 @@ impl IntoExecutionPendingEnvelope for GossipVerifiedEnve let signed_envelope = self.signed_envelope; let envelope = &signed_envelope.message; let payload = &envelope.payload; - + // TODO(gloas) unwrap - let bid = chain.get_full_block(&envelope.beacon_block_root).unwrap().unwrap().message().body().signed_execution_payload_bid().unwrap().message; + let bid = chain + .get_full_block(&envelope.beacon_block_root) + .unwrap() + .unwrap() + .message() + .body() + .signed_execution_payload_bid() + .unwrap() + .message; // Verify the execution payload is valid let payload_notifier = diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs index 4a739e74c5..b166e90fa1 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs @@ -17,6 +17,9 @@ //! |--------------- //! | //! ▼ +//! SignatureVerifiedEnvelope +//! | +//! ▼ //! ExecutionPendingEnvelope //! | //! await @@ -28,15 +31,25 @@ use std::sync::Arc; -use state_processing::{BlockProcessingError, ConsensusContext, envelope_processing::EnvelopeProcessingError}; +use state_processing::{ + BlockProcessingError, ConsensusContext, envelope_processing::EnvelopeProcessingError, +}; use tracing::instrument; -use types::{BeaconState, BeaconStateError, ChainSpec, DataColumnSidecarList, EthSpec, ExecutionBlockHash, Hash256, SignedBeaconBlock, SignedExecutionPayloadEnvelope, Slot}; +use types::{ + BeaconState, BeaconStateError, ChainSpec, DataColumnSidecarList, EthSpec, ExecutionBlockHash, + Hash256, SignedBeaconBlock, SignedExecutionPayloadEnvelope, Slot, +}; -use crate::{BeaconChain, BeaconChainError, BeaconChainTypes, NotifyExecutionLayer, block_verification::PayloadVerificationHandle, payload_envelope_verification::gossip_verified_envelope::GossipVerifiedEnvelope}; +use crate::{ + BeaconChain, BeaconChainError, BeaconChainTypes, NotifyExecutionLayer, + block_verification::PayloadVerificationHandle, + payload_envelope_verification::gossip_verified_envelope::GossipVerifiedEnvelope, +}; pub mod execution_pending_envelope; pub mod gossip_verified_envelope; mod payload_notifier; +mod signature_verified_envelope; pub trait IntoExecutionPendingEnvelope: Sized { fn into_execution_pending_envelope( @@ -115,6 +128,12 @@ pub enum EnvelopeError { committed_bid: ExecutionBlockHash, envelope: ExecutionBlockHash, }, + // The slot belongs to a block that is from a slot prior than + // the most recently finalized slot + PriorToFinalization { + payload_slot: Slot, + latest_finalized_slot: Slot, + }, // Some Beacon Chain Error BeaconChainError(Arc), // Some Beacon State error diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs index 15fec7e21d..6bbdd971a5 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs @@ -2,12 +2,19 @@ use std::sync::Arc; use execution_layer::NewPayloadRequest; use fork_choice::PayloadVerificationStatus; -use state_processing::{envelope_processing::partially_verify_payload_envelope, per_block_processing::is_execution_enabled}; +use state_processing::{ + envelope_processing::partially_verify_payload_envelope, + per_block_processing::is_execution_enabled, +}; use tracing::warn; -use types::{BeaconState, ExecutionPayloadBid, Hash256, SignedBeaconBlock, SignedExecutionPayloadEnvelope}; - -use crate::{BeaconChain, BeaconChainTypes, BlockError, ExecutionPayloadError, NotifyExecutionLayer, execution_payload::notify_new_payload}; +use types::{ + BeaconState, ExecutionPayloadBid, Hash256, SignedBeaconBlock, SignedExecutionPayloadEnvelope, +}; +use crate::{ + BeaconChain, BeaconChainTypes, BlockError, ExecutionPayloadError, NotifyExecutionLayer, + execution_payload::notify_new_payload, +}; /// Used to await the result of executing payload with a remote EE. pub struct PayloadNotifier { @@ -24,7 +31,7 @@ impl PayloadNotifier { state: &BeaconState, notify_execution_layer: NotifyExecutionLayer, ) -> Result { - let payload_verification_status = { + let payload_verification_status = { // Perform the initial stages of payload verification. // // We will duplicate these checks again during `per_block_processing`, however these @@ -32,11 +39,7 @@ impl PayloadNotifier { // the block as optimistically imported. This is particularly relevant in the case // where we do not send the block to the EL at all. let payload_message = &envelope.message; - partially_verify_payload_envelope( - state, - &envelope, - &chain.spec, - ).unwrap(); // TODO(gloas) unwrap + partially_verify_payload_envelope(state, &envelope, &chain.spec).unwrap(); // TODO(gloas) unwrap match notify_execution_layer { NotifyExecutionLayer::No if chain.config.optimistic_finalized_sync => { @@ -71,7 +74,12 @@ impl PayloadNotifier { Ok(precomputed_status) } else { // tODO(gloas) fix zero - notify_new_payload(&self.chain, Hash256::ZERO, self.envelope.message.try_into()?).await + notify_new_payload( + &self.chain, + Hash256::ZERO, + self.envelope.message.try_into()?, + ) + .await } } -} \ No newline at end of file +} diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/signature_verified_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/signature_verified_envelope.rs new file mode 100644 index 0000000000..cd430daf5b --- /dev/null +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/signature_verified_envelope.rs @@ -0,0 +1,40 @@ +use std::sync::Arc; + +use state_processing::ConsensusContext; +use types::{BeaconState, Hash256, SignedExecutionPayloadEnvelope}; + +use crate::{BeaconChain, BeaconChainTypes, payload_envelope_verification::{EnvelopeError, MaybeAvailableEnvelope}}; + + +/// A wrapper around a `SignedExecutionPayloadEnvelope` that indicates that all signatures (except the deposit +/// signatures) have been verified. +pub struct SignatureVerifiedEnvelope { + envelope: SignedExecutionPayloadEnvelope, + block_root: Hash256, + state: Option>, + consensus_context: ConsensusContext, +} + + +impl SignatureVerifiedEnvelope { + pub fn new( + envelope: Arc>, + state: &mut BeaconState, + block_root: Hash256, + chain: &BeaconChain, + ) -> Result { + let is_signature_valid = envelope.verify_signature_with_state(state, &chain.spec)?; + + if !is_signature_valid { + return Err(EnvelopeError::BadSignature) + } + + Self { + envelope, + block_root, + state + } + + todo!() + } +} \ No newline at end of file diff --git a/beacon_node/execution_layer/src/engine_api/new_payload_request.rs b/beacon_node/execution_layer/src/engine_api/new_payload_request.rs index b88dd7ca3a..145d171bbc 100644 --- a/beacon_node/execution_layer/src/engine_api/new_payload_request.rs +++ b/beacon_node/execution_layer/src/engine_api/new_payload_request.rs @@ -4,7 +4,8 @@ use crate::versioned_hashes::verify_versioned_hashes; use state_processing::per_block_processing::deneb::kzg_commitment_to_versioned_hash; use superstruct::superstruct; use types::{ - BeaconBlockRef, BeaconStateError, EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadEnvelope, ExecutionPayloadRef, Hash256, VersionedHash + BeaconBlockRef, BeaconStateError, EthSpec, ExecutionBlockHash, ExecutionPayload, + ExecutionPayloadEnvelope, ExecutionPayloadRef, Hash256, VersionedHash, }; use types::{ ExecutionPayloadBellatrix, ExecutionPayloadCapella, ExecutionPayloadDeneb, diff --git a/consensus/state_processing/src/envelope_processing.rs b/consensus/state_processing/src/envelope_processing.rs index 2f511846a5..0cdb6caaef 100644 --- a/consensus/state_processing/src/envelope_processing.rs +++ b/consensus/state_processing/src/envelope_processing.rs @@ -277,7 +277,6 @@ pub fn process_execution_payload_envelope( Ok(()) } - /// Performs *partial* verification of the `payload envelope`. pub fn partially_verify_payload_envelope( state: &BeaconState, @@ -286,7 +285,7 @@ pub fn partially_verify_payload_envelope( ) -> Result<(), EnvelopeProcessingError> { let envelope = &signed_envelope.message; let payload = &signed_envelope.message.payload; - + // Verify consistency with the beacon block let latest_block_header_root = state.latest_block_header().tree_hash_root(); envelope_verify!( From 9f972d1743d068ba82199ad8b4c9b410927b3fdb Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Wed, 11 Feb 2026 14:53:24 -0800 Subject: [PATCH 04/19] progress --- beacon_node/beacon_chain/src/beacon_chain.rs | 28 +++++ .../beacon_chain/src/execution_payload.rs | 1 - .../gossip_verified_envelope.rs | 102 +++++++++--------- .../src/payload_envelope_verification/mod.rs | 80 ++++++++++++-- .../payload_notifier.rs | 66 +++++++----- .../signature_verified_envelope.rs | 40 ------- .../payload_envelope_verification/tests.rs | 20 ++++ .../src/engine_api/new_payload_request.rs | 2 +- .../src/envelope_processing.rs | 100 ----------------- 9 files changed, 211 insertions(+), 228 deletions(-) delete mode 100644 beacon_node/beacon_chain/src/payload_envelope_verification/signature_verified_envelope.rs create mode 100644 beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 801ef7696f..7b825be2c3 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -56,6 +56,7 @@ use crate::observed_block_producers::ObservedBlockProducers; use crate::observed_data_sidecars::ObservedDataSidecars; use crate::observed_operations::{ObservationOutcome, ObservedOperations}; use crate::observed_slashable::ObservedSlashable; +use crate::payload_envelope_verification::{ExecutedEnvelope, ExecutionPendingEnvelope}; use crate::persisted_beacon_chain::PersistedBeaconChain; use crate::persisted_custody::persist_custody_context; use crate::persisted_fork_choice::PersistedForkChoice; @@ -3547,6 +3548,33 @@ impl BeaconChain { )) } + /// Accepts a fully-verified payload envelope and awaits on its payload verification handle to + /// get a fully `ExecutedEnvelope`. + /// + /// An error is returned if the verification handle couldn't be awaited. + #[instrument(skip_all, level = "debug")] + pub async fn into_executed_payload_envelope( + self: Arc, + pending_envelope: ExecutionPendingEnvelope, + ) -> Result, BlockError> { + let ExecutionPendingEnvelope { + signed_envelope, + import_data, + payload_verification_handle, + } = pending_envelope; + + let payload_verification_outcome = payload_verification_handle + .await + .map_err(BeaconChainError::TokioJoin)? + .ok_or(BeaconChainError::RuntimeShutdown)??; + + Ok(ExecutedEnvelope::new( + signed_envelope, + import_data, + payload_verification_outcome, + )) + } + /* Import methods */ /// Checks if the block is available, and imports immediately if so, otherwise caches the block diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index 631a0180dd..0be058231a 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -7,7 +7,6 @@ //! So, this module contains functions that one might expect to find in other crates, but they live //! here for good reason. -use crate::payload_envelope_verification::EnvelopeError; use crate::{ BeaconChain, BeaconChainError, BeaconChainTypes, BlockError, BlockProductionError, ExecutionPayloadError, diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs index 580c6e9234..a7ccc056ac 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs @@ -4,7 +4,10 @@ use educe::Educe; use slot_clock::SlotClock; use state_processing::{VerifySignatures, envelope_processing::process_execution_payload_envelope}; use tracing::debug; -use types::{EthSpec, SignedBeaconBlock, SignedExecutionPayloadEnvelope, consts::gloas::BUILDER_INDEX_SELF_BUILD}; +use types::{ + EthSpec, SignedBeaconBlock, SignedExecutionPayloadEnvelope, + consts::gloas::BUILDER_INDEX_SELF_BUILD, +}; use crate::{ BeaconChain, BeaconChainError, BeaconChainTypes, NotifyExecutionLayer, @@ -70,7 +73,7 @@ impl GossipVerifiedEnvelope { .signed_execution_payload_bid()? .message; - // check that the envelopes slot isnt from a slot prior + // check that the envelopes slot isnt from a slot prior // to the latest finalized slot. if envelope.slot < latest_finalized_slot { return Err(EnvelopeError::PriorToFinalization { @@ -103,54 +106,58 @@ impl GossipVerifiedEnvelope { }); } - // Get the fork from the proposer cache so we can verify the signature. - // This is currently the most efficient way to implement envelope signature verification - // because the `fork` might depend on advancing the parent state. + // Verify the envelope signature. + // + // For self-build envelopes, we can use the proposer cache for the fork and the + // validator pubkey cache for the proposer's pubkey, avoiding a state load from disk. + // For external builder envelopes, we must load the state to access the builder registry. + let builder_index = envelope.builder_index; let block_slot = envelope.slot; let block_epoch = block_slot.epoch(T::EthSpec::slots_per_epoch()); let proposer_shuffling_decision_block = proto_block.proposer_shuffling_root_for_child_block(block_epoch, &chain.spec); - let mut opt_snapshot = None; - let envelope_ref = signed_envelope.as_ref(); - let proposer = chain.with_proposer_cache::<_, EnvelopeError>( - proposer_shuffling_decision_block, - block_epoch, - |proposers| proposers.get_slot::(block_slot), - || { - debug!( - %beacon_block_root, - block_hash = %envelope_ref.block_hash(), - "Proposer shuffling cache miss for envelope verification" - ); - // The proposer index was *not* cached and we must load the parent in order to - // determine the proposer index. - let snapshot = load_snapshot(envelope_ref, chain)?; - opt_snapshot = Some(Box::new(snapshot.clone())); - Ok((snapshot.state_root, snapshot.pre_state)) - }, - )?; - let fork = proposer.fork; - let builder_index = envelope.builder_index; - let index = if builder_index == BUILDER_INDEX_SELF_BUILD { - block.message().proposer_index() - } else { - builder_index - }; + let (signature_is_valid, opt_snapshot) = if builder_index == BUILDER_INDEX_SELF_BUILD { + // Fast path: self-build envelopes can be verified without loading the state. + let envelope_ref = signed_envelope.as_ref(); + let mut opt_snapshot = None; + let proposer = chain.with_proposer_cache::<_, EnvelopeError>( + proposer_shuffling_decision_block, + block_epoch, + |proposers| proposers.get_slot::(block_slot), + || { + debug!( + %beacon_block_root, + "Proposer shuffling cache miss for envelope verification" + ); + let snapshot = load_snapshot(envelope_ref, chain)?; + opt_snapshot = Some(Box::new(snapshot.clone())); + Ok((snapshot.state_root, snapshot.pre_state)) + }, + )?; + let fork = proposer.fork; - let signature_is_valid = { - // TODO(gloas) the builder pubkey wont be in the validator pubkey cache - // this will currently only work for local block building. let pubkey_cache = chain.validator_pubkey_cache.read(); let pubkey = pubkey_cache - .get(index as usize) - .ok_or_else(|| EnvelopeError::UnknownValidator { builder_index: index })?; - signed_envelope.verify_signature( + .get(block.message().proposer_index() as usize) + .ok_or_else(|| EnvelopeError::UnknownValidator { + builder_index: block.message().proposer_index(), + })?; + let is_valid = signed_envelope.verify_signature( pubkey, &fork, chain.genesis_validators_root, &chain.spec, - ) + ); + (is_valid, opt_snapshot) + } else { + // TODO(gloas) we should probably introduce a builder cache or some type of + // global cache. + // External builder: must load the state to get the builder pubkey. + let snapshot = load_snapshot(signed_envelope.as_ref(), chain)?; + let is_valid = + signed_envelope.verify_signature_with_state(&snapshot.pre_state, &chain.spec)?; + (is_valid, Some(Box::new(snapshot))) }; if !signature_is_valid { @@ -179,20 +186,13 @@ impl IntoExecutionPendingEnvelope for GossipVerifiedEnve let envelope = &signed_envelope.message; let payload = &envelope.payload; - // TODO(gloas) unwrap - let bid = chain - .get_full_block(&envelope.beacon_block_root) - .unwrap() - .unwrap() - .message() - .body() - .signed_execution_payload_bid() - .unwrap() - .message; - // Verify the execution payload is valid - let payload_notifier = - PayloadNotifier::new(chain.clone(), envelope, notify_execution_layer)?; + let payload_notifier = PayloadNotifier::new( + chain.clone(), + signed_envelope.clone(), + self.block.clone(), + notify_execution_layer, + )?; let block_root = envelope.beacon_block_root; let slot = self.block.slot(); diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs index b166e90fa1..7ddc4b5c64 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs @@ -17,9 +17,6 @@ //! |--------------- //! | //! ▼ -//! SignatureVerifiedEnvelope -//! | -//! ▼ //! ExecutionPendingEnvelope //! | //! await @@ -41,15 +38,16 @@ use types::{ }; use crate::{ - BeaconChain, BeaconChainError, BeaconChainTypes, NotifyExecutionLayer, - block_verification::PayloadVerificationHandle, + AvailabilityProcessingStatus, BeaconChain, BeaconChainError, BeaconChainTypes, + ExecutionPayloadError, NotifyExecutionLayer, PayloadVerificationOutcome, + block_verification::PayloadVerificationHandle, block_verification_types::BlockImportData, payload_envelope_verification::gossip_verified_envelope::GossipVerifiedEnvelope, }; pub mod execution_pending_envelope; pub mod gossip_verified_envelope; mod payload_notifier; -mod signature_verified_envelope; +mod tests; pub trait IntoExecutionPendingEnvelope: Sized { fn into_execution_pending_envelope( @@ -101,7 +99,67 @@ pub struct EnvelopeProcessingSnapshot { pub beacon_block_root: Hash256, } -#[derive(Debug, Clone)] +/// A payload envelope that has gone through processing checks and execution by an EL client. +/// This envelope hasn't necessarily completed data availability checks. +/// +/// +/// It contains 2 variants: +/// 1. `Available`: This enelope has been executed and also contains all data to consider it +/// fully available. +/// 2. `AvailabilityPending`: This envelope hasn't received all required blobs to consider it +/// fully available. +pub enum ExecutedEnvelope { + Available(AvailableExecutedEnvelope), + // TODO(gloas) implement availability pending + AvailabilityPending(), +} + +impl ExecutedEnvelope { + pub fn new( + envelope: MaybeAvailableEnvelope, + import_data: EnvelopeImportData, + payload_verification_outcome: PayloadVerificationOutcome, + ) -> Self { + match envelope { + MaybeAvailableEnvelope::Available(available_envelope) => { + Self::Available(AvailableExecutedEnvelope::new( + available_envelope, + import_data, + payload_verification_outcome, + )) + } + // TODO(gloas) implement availability pending + MaybeAvailableEnvelope::AvailabilityPending { + block_hash: _, + envelope: _, + } => Self::AvailabilityPending(), + } + } +} + +/// A payload envelope that has completed all payload processing checks including verification +/// by an EL client **and** has all requisite blob data to be imported into fork choice. +pub struct AvailableExecutedEnvelope { + pub envelope: AvailableEnvelope, + pub import_data: EnvelopeImportData, + pub payload_verification_outcome: PayloadVerificationOutcome, +} + +impl AvailableExecutedEnvelope { + pub fn new( + envelope: AvailableEnvelope, + import_data: EnvelopeImportData, + payload_verification_outcome: PayloadVerificationOutcome, + ) -> Self { + Self { + envelope, + import_data, + payload_verification_outcome, + } + } +} + +#[derive(Debug)] pub enum EnvelopeError { /// The envelope's block root is unknown. BlockRootUnknown { @@ -142,6 +200,8 @@ pub enum EnvelopeError { BlockProcessingError(BlockProcessingError), // Some EnvelopeProcessingError EnvelopeProcessingError(EnvelopeProcessingError), + // Error verifying the execution payload + ExecutionPayloadError(ExecutionPayloadError), } impl From for EnvelopeError { @@ -150,6 +210,12 @@ impl From for EnvelopeError { } } +impl From for EnvelopeError { + fn from(e: ExecutionPayloadError) -> Self { + EnvelopeError::ExecutionPayloadError(e) + } +} + impl From for EnvelopeError { fn from(e: BeaconStateError) -> Self { EnvelopeError::BeaconStateError(e) diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs index 6bbdd971a5..b485af0bfa 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs @@ -1,15 +1,10 @@ use std::sync::Arc; -use execution_layer::NewPayloadRequest; +use execution_layer::{NewPayloadRequest, NewPayloadRequestGloas}; use fork_choice::PayloadVerificationStatus; -use state_processing::{ - envelope_processing::partially_verify_payload_envelope, - per_block_processing::is_execution_enabled, -}; +use state_processing::per_block_processing::deneb::kzg_commitment_to_versioned_hash; use tracing::warn; -use types::{ - BeaconState, ExecutionPayloadBid, Hash256, SignedBeaconBlock, SignedExecutionPayloadEnvelope, -}; +use types::{SignedBeaconBlock, SignedExecutionPayloadEnvelope}; use crate::{ BeaconChain, BeaconChainTypes, BlockError, ExecutionPayloadError, NotifyExecutionLayer, @@ -19,33 +14,26 @@ use crate::{ /// Used to await the result of executing payload with a remote EE. pub struct PayloadNotifier { pub chain: Arc>, - pub envelope: Arc>, + envelope: Arc>, + block: Arc>, payload_verification_status: Option, } impl PayloadNotifier { pub fn new( chain: Arc>, - bid: &ExecutionPayloadBid, envelope: Arc>, - state: &BeaconState, + block: Arc>, notify_execution_layer: NotifyExecutionLayer, ) -> Result { let payload_verification_status = { - // Perform the initial stages of payload verification. - // - // We will duplicate these checks again during `per_block_processing`, however these - // checks are cheap and doing them here ensures we have verified them before marking - // the block as optimistically imported. This is particularly relevant in the case - // where we do not send the block to the EL at all. let payload_message = &envelope.message; - partially_verify_payload_envelope(state, &envelope, &chain.spec).unwrap(); // TODO(gloas) unwrap match notify_execution_layer { NotifyExecutionLayer::No if chain.config.optimistic_finalized_sync => { - // Create a NewPayloadRequest (no clones required) and check optimistic sync verifications - let new_payload_request: NewPayloadRequest = - payload_message.try_into()?; + // TODO(gloas) unwrap + let new_payload_request = + Self::build_new_payload_request(&envelope, &block).unwrap(); if let Err(e) = new_payload_request.perform_optimistic_sync_verifications() { warn!( block_number = ?payload_message.payload.block_number, @@ -65,6 +53,7 @@ impl PayloadNotifier { Ok(Self { chain, envelope, + block, payload_verification_status, }) } @@ -73,13 +62,34 @@ impl PayloadNotifier { if let Some(precomputed_status) = self.payload_verification_status { Ok(precomputed_status) } else { - // tODO(gloas) fix zero - notify_new_payload( - &self.chain, - Hash256::ZERO, - self.envelope.message.try_into()?, - ) - .await + let block_root = self.envelope.message.beacon_block_root; + let request = Self::build_new_payload_request(&self.envelope, &self.block)?; + notify_new_payload(&self.chain, block_root, request).await } } + + fn build_new_payload_request<'a>( + envelope: &'a SignedExecutionPayloadEnvelope, + block: &'a SignedBeaconBlock, + ) -> Result, BlockError> { + let bid = &block + .message() + .body() + .signed_execution_payload_bid() + .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))? + .message; + + let versioned_hashes = bid + .blob_kzg_commitments + .iter() + .map(kzg_commitment_to_versioned_hash) + .collect(); + + Ok(NewPayloadRequest::Gloas(NewPayloadRequestGloas { + execution_payload: &envelope.message.payload, + versioned_hashes, + parent_beacon_block_root: block.message().parent_root(), + execution_requests: &envelope.message.execution_requests, + })) + } } diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/signature_verified_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/signature_verified_envelope.rs deleted file mode 100644 index cd430daf5b..0000000000 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/signature_verified_envelope.rs +++ /dev/null @@ -1,40 +0,0 @@ -use std::sync::Arc; - -use state_processing::ConsensusContext; -use types::{BeaconState, Hash256, SignedExecutionPayloadEnvelope}; - -use crate::{BeaconChain, BeaconChainTypes, payload_envelope_verification::{EnvelopeError, MaybeAvailableEnvelope}}; - - -/// A wrapper around a `SignedExecutionPayloadEnvelope` that indicates that all signatures (except the deposit -/// signatures) have been verified. -pub struct SignatureVerifiedEnvelope { - envelope: SignedExecutionPayloadEnvelope, - block_root: Hash256, - state: Option>, - consensus_context: ConsensusContext, -} - - -impl SignatureVerifiedEnvelope { - pub fn new( - envelope: Arc>, - state: &mut BeaconState, - block_root: Hash256, - chain: &BeaconChain, - ) -> Result { - let is_signature_valid = envelope.verify_signature_with_state(state, &chain.spec)?; - - if !is_signature_valid { - return Err(EnvelopeError::BadSignature) - } - - Self { - envelope, - block_root, - state - } - - todo!() - } -} \ No newline at end of file diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs new file mode 100644 index 0000000000..381a5eaa4f --- /dev/null +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs @@ -0,0 +1,20 @@ +use std::sync::Arc; + +use crate::{AvailabilityProcessingStatus, BeaconChain, BeaconChainTypes, payload_envelope_verification::{ExecutedEnvelope, ExecutionPendingEnvelope}}; + +async fn import_execution_pending_envelope( + chain: Arc>, + execution_pending_envelope: ExecutionPendingEnvelope, +) -> Result { + match chain + .clone() + .into_executed_payload_envelope(execution_pending_envelope) + .await + .unwrap() + { + ExecutedEnvelope::Available(envelope) => todo!(), + ExecutedEnvelope::AvailabilityPending() => { + Err("AvailabilityPending not expected in this test. Block not imported.".to_string()) + } + } +} diff --git a/beacon_node/execution_layer/src/engine_api/new_payload_request.rs b/beacon_node/execution_layer/src/engine_api/new_payload_request.rs index 145d171bbc..ba94296b85 100644 --- a/beacon_node/execution_layer/src/engine_api/new_payload_request.rs +++ b/beacon_node/execution_layer/src/engine_api/new_payload_request.rs @@ -5,7 +5,7 @@ use state_processing::per_block_processing::deneb::kzg_commitment_to_versioned_h use superstruct::superstruct; use types::{ BeaconBlockRef, BeaconStateError, EthSpec, ExecutionBlockHash, ExecutionPayload, - ExecutionPayloadEnvelope, ExecutionPayloadRef, Hash256, VersionedHash, + ExecutionPayloadRef, Hash256, VersionedHash, }; use types::{ ExecutionPayloadBellatrix, ExecutionPayloadCapella, ExecutionPayloadDeneb, diff --git a/consensus/state_processing/src/envelope_processing.rs b/consensus/state_processing/src/envelope_processing.rs index 0cdb6caaef..d46728dbbc 100644 --- a/consensus/state_processing/src/envelope_processing.rs +++ b/consensus/state_processing/src/envelope_processing.rs @@ -276,103 +276,3 @@ pub fn process_execution_payload_envelope( Ok(()) } - -/// Performs *partial* verification of the `payload envelope`. -pub fn partially_verify_payload_envelope( - state: &BeaconState, - signed_envelope: &SignedExecutionPayloadEnvelope, - spec: &ChainSpec, -) -> Result<(), EnvelopeProcessingError> { - let envelope = &signed_envelope.message; - let payload = &signed_envelope.message.payload; - - // Verify consistency with the beacon block - let latest_block_header_root = state.latest_block_header().tree_hash_root(); - envelope_verify!( - envelope.beacon_block_root == latest_block_header_root, - EnvelopeProcessingError::LatestBlockHeaderMismatch { - envelope_root: envelope.beacon_block_root, - block_header_root: latest_block_header_root, - } - ); - envelope_verify!( - envelope.slot == state.slot(), - EnvelopeProcessingError::SlotMismatch { - envelope_slot: envelope.slot, - parent_state_slot: state.slot(), - } - ); - - // Verify consistency with the committed bid - let committed_bid = state.latest_execution_payload_bid()?; - envelope_verify!( - envelope.builder_index == committed_bid.builder_index, - EnvelopeProcessingError::BuilderIndexMismatch { - committed_bid: committed_bid.builder_index, - envelope: envelope.builder_index, - } - ); - envelope_verify!( - committed_bid.prev_randao == payload.prev_randao, - EnvelopeProcessingError::PrevRandaoMismatch { - committed_bid: committed_bid.prev_randao, - envelope: payload.prev_randao, - } - ); - - // Verify consistency with expected withdrawals - // NOTE: we don't bother hashing here except in case of error, because we can just compare for - // equality directly. This equality check could be more straight-forward if the types were - // changed to match (currently we are comparing VariableList to List). This could happen - // coincidentally when we adopt ProgressiveList. - envelope_verify!( - payload.withdrawals.len() == state.payload_expected_withdrawals()?.len() - && payload - .withdrawals - .iter() - .eq(state.payload_expected_withdrawals()?.iter()), - EnvelopeProcessingError::WithdrawalsRootMismatch { - state: state.payload_expected_withdrawals()?.tree_hash_root(), - payload: payload.withdrawals.tree_hash_root(), - } - ); - - // Verify the gas limit - envelope_verify!( - committed_bid.gas_limit == payload.gas_limit, - EnvelopeProcessingError::GasLimitMismatch { - committed_bid: committed_bid.gas_limit, - envelope: payload.gas_limit, - } - ); - - // Verify the block hash - envelope_verify!( - committed_bid.block_hash == payload.block_hash, - EnvelopeProcessingError::BlockHashMismatch { - committed_bid: committed_bid.block_hash, - envelope: payload.block_hash, - } - ); - - // Verify consistency of the parent hash with respect to the previous execution payload - envelope_verify!( - payload.parent_hash == *state.latest_block_hash()?, - EnvelopeProcessingError::ParentHashMismatch { - state: *state.latest_block_hash()?, - envelope: payload.parent_hash, - } - ); - - // Verify timestamp - let state_timestamp = compute_timestamp_at_slot(state, state.slot(), spec)?; - envelope_verify!( - payload.timestamp == state_timestamp, - EnvelopeProcessingError::TimestampMismatch { - state: state_timestamp, - envelope: payload.timestamp, - } - ); - - Ok(()) -} From f637a68e04e9ca3a810245a38cc67ff3475e30ef Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Wed, 11 Feb 2026 23:34:53 -0800 Subject: [PATCH 05/19] Import payload flow --- beacon_node/beacon_chain/src/beacon_chain.rs | 6 +- .../beacon_chain/src/execution_payload.rs | 6 +- beacon_node/beacon_chain/src/lib.rs | 1 + .../src/payload_envelope_import/mod.rs | 339 ++++++++++++++++++ .../execution_pending_envelope.rs | 28 -- .../gossip_verified_envelope.rs | 65 +++- .../src/payload_envelope_verification/mod.rs | 58 ++- .../payload_notifier.rs | 2 +- .../payload_envelope_verification/tests.rs | 7 +- .../beacon_chain/src/validator_monitor.rs | 2 +- .../gossip_methods.rs | 145 +++++++- 11 files changed, 599 insertions(+), 60 deletions(-) create mode 100644 beacon_node/beacon_chain/src/payload_envelope_import/mod.rs delete mode 100644 beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 7b825be2c3..0c10aaadc4 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -3555,7 +3555,7 @@ impl BeaconChain { #[instrument(skip_all, level = "debug")] pub async fn into_executed_payload_envelope( self: Arc, - pending_envelope: ExecutionPendingEnvelope, + pending_envelope: ExecutionPendingEnvelope, ) -> Result, BlockError> { let ExecutionPendingEnvelope { signed_envelope, @@ -4168,7 +4168,7 @@ impl BeaconChain { Ok(block_root) } - fn handle_import_block_db_write_error( + pub(crate) fn handle_import_block_db_write_error( &self, // We don't actually need this value, however it's always present when we call this function // and it needs to be dropped to prevent a dead-lock. Requiring it to be passed here is @@ -4202,7 +4202,7 @@ impl BeaconChain { } /// Check block's consistentency with any configured weak subjectivity checkpoint. - fn check_block_against_weak_subjectivity_checkpoint( + pub(crate) fn check_block_against_weak_subjectivity_checkpoint( &self, block: BeaconBlockRef, block_root: Hash256, diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index 0be058231a..b0644ac8aa 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -160,7 +160,8 @@ pub async fn notify_new_payload( ?validation_error, ?latest_valid_hash, ?execution_block_hash, - // root = ?block.tree_hash_root(), + // TODO(gloas) are these other logs important? + root = ?beacon_block_root, // graffiti = block.body().graffiti().as_utf8_lossy(), // proposer_index = block.proposer_index(), // slot = %block.slot(), @@ -205,7 +206,8 @@ pub async fn notify_new_payload( warn!( ?validation_error, ?execution_block_hash, - // root = ?block.tree_hash_root(), + // TODO(gloas) are these other logs important? + root = ?beacon_block_root, // graffiti = block.body().graffiti().as_utf8_lossy(), // proposer_index = block.proposer_index(), // slot = %block.slot(), diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index 4120ed86fc..f0151eed01 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -42,6 +42,7 @@ pub mod observed_block_producers; pub mod observed_data_sidecars; pub mod observed_operations; mod observed_slashable; +pub mod payload_envelope_import; pub mod payload_envelope_verification; pub mod persisted_beacon_chain; pub mod persisted_custody; diff --git a/beacon_node/beacon_chain/src/payload_envelope_import/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_import/mod.rs new file mode 100644 index 0000000000..ab7de5c15d --- /dev/null +++ b/beacon_node/beacon_chain/src/payload_envelope_import/mod.rs @@ -0,0 +1,339 @@ +use std::sync::Arc; + +use fork_choice::PayloadVerificationStatus; +use logging::crit; +use store::StoreOp; +use tracing::{debug, error, info_span, instrument}; +use types::{BeaconState, BlockImportSource, EthSpec, Hash256, SignedBeaconBlock}; + +use crate::{ + AvailabilityProcessingStatus, BeaconChain, BeaconChainError, BeaconChainTypes, BlockError, + NotifyExecutionLayer, + block_verification_types::{AsBlock, AvailableBlockData}, + payload_envelope_verification::{ + AvailableEnvelope, AvailableExecutedEnvelope, EnvelopeImportData, ExecutedEnvelope, + IntoExecutionPendingEnvelope, + }, + validator_monitor::timestamp_now, +}; + +impl BeaconChain { + /// Returns `Ok(block_root)` if the given `unverified_envelope` was successfully verified and + /// imported into the chain. + /// + /// Items that implement `IntoExecutionPendingEnvelope` include: + /// + /// - `GossipVerifiedEnvelope` + /// + /// ## Errors + /// + /// Returns an `Err` if the given block was invalid, or an error was encountered during + /// verification. + #[instrument(skip_all, fields(block_root = ?block_root, block_source = %block_source))] + pub async fn process_execution_payload_envelope>( + self: &Arc, + block_root: Hash256, + unverified_envelope: P, + notify_execution_layer: NotifyExecutionLayer, + block_source: BlockImportSource, + publish_fn: impl FnOnce() -> Result<(), BlockError>, + ) -> Result { + let block_slot = unverified_envelope.envelope().slot(); + + // TODO(gloas) Set observed time if not already set. Usually this should be set by gossip or RPC, + // but just in case we set it again here (useful for tests). + + // TODO(gloas) if we decide to insert the payload envelope into the new DA checker + // we should insert the pre executed envelope here. + + // TODO(gloas) Start the Prometheus timer. + // let _full_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_TIMES); + + // TODO(gloas) Increment the Prometheus counter for envelope processing requests. + // metrics::inc_counter(&metrics::BLOCK_PROCESSING_REQUESTS); + + // A small closure to group the verification and import errors. + let chain = self.clone(); + let import_envelope = async move { + let execution_pending = unverified_envelope + .into_execution_pending_envelope(&chain, notify_execution_layer)?; + publish_fn()?; + + // TODO(gloas) Record the time it took to complete consensus verification. + // if let Some(timestamp) = self.slot_clock.now_duration() { + // self.block_times_cache + // .write() + // .set_time_consensus_verified(block_root, block_slot, timestamp) + // } + + let executed_envelope = chain + .into_executed_payload_envelope(execution_pending) + .await + .inspect_err(|_| { + // TODO(gloas) If the envelope fails execution for whatever reason (e.g. engine offline), + // and we keep it in the cache, then the node will NOT perform lookup and + // reprocess this block until the block is evicted from DA checker, causing the + // chain to get stuck temporarily if the block is canonical. Therefore we remove + // it from the cache if execution fails. + + //self.data_availability_checker + // .remove_block_on_execution_error(&block_root); + })?; + + // TODO(gloas) Record the *additional* time it took to wait for execution layer verification. + // if let Some(timestamp) = self.slot_clock.now_duration() { + // self.block_times_cache + // .write() + // .set_time_executed(block_root, block_slot, timestamp) + // } + + match executed_envelope { + ExecutedEnvelope::Available(envelope) => { + self.import_available_execution_payload_envelope(Box::new(envelope)) + .await + } + ExecutedEnvelope::AvailabilityPending() => { + return Err(BlockError::InternalError( + "Pending payload envelope not yet implemented".to_owned(), + )); + } + } + }; + + // Verify and import the block. + match import_envelope.await { + // The block was successfully verified and imported. Yay. + Ok(status @ AvailabilityProcessingStatus::Imported(block_root)) => { + debug!( + ?block_root, + %block_slot, + source = %block_source, + "Envelope imported" + ); + + // TODO(gloas) Increment the Prometheus counter for block processing successes. + // metrics::inc_counter(&metrics::BLOCK_PROCESSING_SUCCESSES); + + Ok(status) + } + Ok(status @ AvailabilityProcessingStatus::MissingComponents(slot, block_root)) => { + debug!(?block_root, %slot, "Beacon block awaiting blobs"); + + Ok(status) + } + Err(BlockError::BeaconChainError(e)) => { + match e.as_ref() { + BeaconChainError::TokioJoin(e) => { + debug!( + error = ?e, + "Envelope processing cancelled" + ); + } + _ => { + // There was an error whilst attempting to verify and import the block. The block might + // be partially verified or partially imported. + crit!( + error = ?e, + "Envelope processing error" + ); + } + }; + Err(BlockError::BeaconChainError(e)) + } + // The block failed verification. + Err(other) => { + debug!(reason = other.to_string(), " Envelope rejected"); + Err(other) + } + } + } + + #[instrument(skip_all)] + pub async fn import_available_execution_payload_envelope( + self: &Arc, + envelope: Box>, + ) -> Result { + let AvailableExecutedEnvelope { + envelope, + import_data, + payload_verification_outcome, + } = *envelope; + + let EnvelopeImportData { + block_root, + block, + post_state, + } = import_data; + + // TODO(gloas) Record the time at which this block's blobs became available. + + let block_root = { + // Capture the current span before moving into the blocking task + let current_span = tracing::Span::current(); + let chain = self.clone(); + self.spawn_blocking_handle( + move || { + // Enter the captured span in the blocking thread + let _guard = current_span.enter(); + chain.import_execution_payload_envelope( + envelope, + block_root, + *post_state, + payload_verification_outcome.payload_verification_status, + block, + ) + }, + "payload_verification_handle", + ) + .await?? + }; + + Ok(AvailabilityProcessingStatus::Imported(block_root)) + } + + /// Accepts a fully-verified and available envelope and imports it into the chain without performing any + /// additional verification. + /// + /// An error is returned if the envelope was unable to be imported. It may be partially imported + /// (i.e., this function is not atomic). + #[allow(clippy::too_many_arguments)] + #[instrument(skip_all)] + fn import_execution_payload_envelope( + &self, + signed_envelope: AvailableEnvelope, + block_root: Hash256, + mut state: BeaconState, + payload_verification_status: PayloadVerificationStatus, + parent_block: Arc>, + ) -> Result { + // ----------------------------- ENVELOPE NOT YET ATTESTABLE ---------------------------------- + // Everything in this initial section is on the hot path between processing the envelope and + // being able to attest to it. DO NOT add any extra processing in this initial section + // unless it must run before fork choice. + // ----------------------------------------------------------------------------------------- + let current_slot = self.slot()?; + let current_epoch = current_slot.epoch(T::EthSpec::slots_per_epoch()); + let envelope = signed_envelope.message(); + + // TODO(gloas) implement metrics + // let post_exec_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_POST_EXEC_PROCESSING); + + // Check the payloads parent block against weak subjectivity checkpoint. + self.check_block_against_weak_subjectivity_checkpoint( + parent_block.message(), + block_root, + &state, + )?; + + // Take an upgradable read lock on fork choice so we can check if this block has already + // been imported. We don't want to repeat work importing a block that is already imported. + let fork_choice_reader = self.canonical_head.fork_choice_upgradable_read_lock(); + if fork_choice_reader.contains_block(&block_root) { + return Err(BlockError::DuplicateFullyImported(block_root)); + } + + // Take an exclusive write-lock on fork choice. It's very important to prevent deadlocks by + // avoiding taking other locks whilst holding this lock. + let mut fork_choice = parking_lot::RwLockUpgradableReadGuard::upgrade(fork_choice_reader); + + // TODO(gloas) Do we need this check? Do not import a block that doesn't descend from the finalized root. + // let signed_block = check_block_is_finalized_checkpoint_or_descendant(self, &fork_choice, signed_block)?; + + // TODO(gloas)Do we want to use an early attester cache like mechanism for payload enevelopes? + // TODO(gloas) emit SSE event if the payload became the new head payload + // drop(post_exec_timer); + + // ---------------------------- BLOCK PROBABLY ATTESTABLE ---------------------------------- + // Most blocks are now capable of being attested to thanks to the `early_attester_cache` + // cache above. Resume non-essential processing. + // + // It is important NOT to return errors here before the database commit, because the block + // has already been added to fork choice and the database would be left in an inconsistent + // state if we returned early without committing. In other words, an error here would + // corrupt the node's database permanently. + // ----------------------------------------------------------------------------------------- + + // Store the envelope and its state, and execute the confirmation batch for the intermediate + // states, which will delete their temporary flags. + // If the write fails, revert fork choice to the version from disk, else we can + // end up with envelopes in fork choice that are missing from disk. + // See https://github.com/sigp/lighthouse/issues/2028 + let (signed_envelope, columns) = signed_envelope.deconstruct(); + + let mut ops = vec![]; + + match self.get_blobs_or_columns_store_op( + block_root, + signed_envelope.slot(), + AvailableBlockData::DataColumns(columns), + ) { + Ok(Some(blobs_or_columns_store_op)) => { + ops.push(blobs_or_columns_store_op); + } + Ok(None) => {} + Err(e) => { + error!( + msg = "Restoring fork choice from disk", + error = &e, + ?block_root, + "Failed to store data columns into the database" + ); + return Err(self + .handle_import_block_db_write_error(fork_choice) + .err() + .unwrap_or(BlockError::InternalError(e))); + } + } + + // TODO(gloas) metrics + // let db_write_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_DB_WRITE); + + ops.push(StoreOp::PutPayloadEnvelope( + block_root, + signed_envelope.clone(), + )); + ops.push(StoreOp::PutState( + signed_envelope.message.state_root, + &state, + )); + + let db_span = info_span!("persist_payloads_and_blobs").entered(); + + // TODO(gloas) do i need this + if let Err(e) = self.store.do_atomically_with_block_and_blobs_cache(ops) { + error!( + msg = "Restoring fork choice from disk", + error = ?e, + "Database write failed!" + ); + // TODO(gloas) handle db write failure + // return Err(self + // .handle_import_block_db_write_error(fork_choice) + // .err() + // .unwrap_or(e.into())); + } + + drop(db_span); + + // The fork choice write-lock is dropped *after* the on-disk database has been updated. + // This prevents inconsistency between the two at the expense of concurrency. + drop(fork_choice); + + // We're declaring the envelope "imported" at this point, since fork choice and the DB know + // about it. + let envelope_time_imported = timestamp_now(); + + // TODO(gloas) depending on what happens with light clients + // we might need to do some computations here + + // TODO(gloas) metrics + // metrics::stop_timer(db_write_timer); + + // TODO(gloas) metrics + // metrics::inc_counter(&metrics::BLOCK_PROCESSING_SUCCESSES); + + // TODO(gloas) we might want to implement something similar + // to `import_block_update_metrics_and_events` + Ok(block_root) + } +} diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs deleted file mode 100644 index 7dffd1c09c..0000000000 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs +++ /dev/null @@ -1,28 +0,0 @@ -use task_executor::JoinHandle; -use types::{EthSpec, FullPayload}; - -use crate::{ - BeaconChainTypes, PayloadVerificationOutcome, - payload_envelope_verification::{MaybeAvailableEnvelope, PayloadEnvelopeImportData}, -}; - -/// Used to await the result of executing payload with an EE. -pub type PayloadVerificationHandle = - JoinHandle>>>; - -/// A wrapper around a `SignedBeaconBlock` that indicates that this block is fully verified and -/// ready to import into the `BeaconChain`. The validation includes: -/// -/// - Parent is known -/// - Signatures -/// - State root check -/// - Block processing -/// -/// Note: a `ExecutionPendingEnvelope` is not _forever_ valid to be imported, it may later become invalid -/// due to finality or some other event. A `ExecutionPendingEnvelope` should be imported into the -/// `BeaconChain` immediately after it is instantiated. -pub struct ExecutionPendingEnvelope { - pub block: MaybeAvailableEnvelope, - pub import_data: PayloadEnvelopeImportData, - pub payload_verification_handle: PayloadVerificationHandle, -} diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs index a7ccc056ac..5af6fe3984 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs @@ -3,14 +3,14 @@ use std::sync::Arc; use educe::Educe; use slot_clock::SlotClock; use state_processing::{VerifySignatures, envelope_processing::process_execution_payload_envelope}; -use tracing::debug; +use tracing::{Span, debug}; use types::{ EthSpec, SignedBeaconBlock, SignedExecutionPayloadEnvelope, consts::gloas::BUILDER_INDEX_SELF_BUILD, }; use crate::{ - BeaconChain, BeaconChainError, BeaconChainTypes, NotifyExecutionLayer, + BeaconChain, BeaconChainError, BeaconChainTypes, BlockError, NotifyExecutionLayer, PayloadVerificationOutcome, payload_envelope_verification::{ EnvelopeError, EnvelopeImportData, EnvelopeProcessingSnapshot, ExecutionPendingEnvelope, @@ -181,7 +181,7 @@ impl IntoExecutionPendingEnvelope for GossipVerifiedEnve self, chain: &Arc>, notify_execution_layer: NotifyExecutionLayer, - ) -> Result, EnvelopeError> { + ) -> Result, BlockError> { let signed_envelope = self.signed_envelope; let envelope = &signed_envelope.message; let payload = &envelope.payload; @@ -255,4 +255,63 @@ impl IntoExecutionPendingEnvelope for GossipVerifiedEnve payload_verification_handle, }) } + + fn envelope(&self) -> &Arc> { + &self.signed_envelope + } +} + +impl BeaconChain { + /// Returns `Ok(GossipVerifiedEnvelope)` if the supplied `envelope` should be forwarded onto the + /// gossip network. The envelope is not imported into the chain, it is just partially verified. + /// + /// The returned `GossipVerifiedEnvelope` should be provided to `Self::process_execution_payload_envelope` immediately + /// after it is returned, unless some other circumstance decides it should not be imported at + /// all. + /// + /// ## Errors + /// + /// Returns an `Err` if the given envelope was invalid, or an error was encountered during + pub async fn verify_envelope_for_gossip( + self: &Arc, + envelope: Arc>, + ) -> Result, EnvelopeError> { + let chain = self.clone(); + let span = Span::current(); + self.task_executor + .clone() + .spawn_blocking_handle( + move || { + let _guard = span.enter(); + let slot = envelope.slot(); + let beacon_block_root = envelope.message.beacon_block_root; + + match GossipVerifiedEnvelope::new(envelope, &chain) { + Ok(verified) => { + debug!( + %slot, + ?beacon_block_root, + "Successfully verified gossip envelope" + ); + + Ok(verified) + } + Err(e) => { + debug!( + error = e.to_string(), + ?beacon_block_root, + %slot, + "Rejected gossip envelope" + ); + + Err(e) + } + } + }, + "gossip_envelope_verification_handle", + ) + .ok_or(BeaconChainError::RuntimeShutdown)? + .await + .map_err(BeaconChainError::TokioJoin)? + } } diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs index 7ddc4b5c64..4dd8d351d7 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs @@ -34,17 +34,16 @@ use state_processing::{ use tracing::instrument; use types::{ BeaconState, BeaconStateError, ChainSpec, DataColumnSidecarList, EthSpec, ExecutionBlockHash, - Hash256, SignedBeaconBlock, SignedExecutionPayloadEnvelope, Slot, + ExecutionPayloadEnvelope, Hash256, SignedBeaconBlock, SignedExecutionPayloadEnvelope, Slot, }; use crate::{ - AvailabilityProcessingStatus, BeaconChain, BeaconChainError, BeaconChainTypes, - ExecutionPayloadError, NotifyExecutionLayer, PayloadVerificationOutcome, - block_verification::PayloadVerificationHandle, block_verification_types::BlockImportData, + BeaconChain, BeaconChainError, BeaconChainTypes, BlockError, ExecutionPayloadError, + NotifyExecutionLayer, PayloadVerificationOutcome, + block_verification::PayloadVerificationHandle, payload_envelope_verification::gossip_verified_envelope::GossipVerifiedEnvelope, }; -pub mod execution_pending_envelope; pub mod gossip_verified_envelope; mod payload_notifier; mod tests; @@ -54,12 +53,14 @@ pub trait IntoExecutionPendingEnvelope: Sized { self, chain: &Arc>, notify_execution_layer: NotifyExecutionLayer, - ) -> Result, EnvelopeError>; + ) -> Result, BlockError>; + + fn envelope(&self) -> &Arc>; } -pub struct ExecutionPendingEnvelope { - pub signed_envelope: MaybeAvailableEnvelope, - pub import_data: EnvelopeImportData, +pub struct ExecutionPendingEnvelope { + pub signed_envelope: MaybeAvailableEnvelope, + pub import_data: EnvelopeImportData, pub payload_verification_handle: PayloadVerificationHandle, } @@ -82,6 +83,25 @@ pub struct AvailableEnvelope { pub spec: Arc, } +impl AvailableEnvelope { + pub fn message(&self) -> &ExecutionPayloadEnvelope { + &self.envelope.message + } + + #[allow(clippy::type_complexity)] + pub fn deconstruct( + self, + ) -> ( + Arc>, + DataColumnSidecarList, + ) { + let AvailableEnvelope { + envelope, columns, .. + } = self; + (envelope, columns) + } +} + pub enum MaybeAvailableEnvelope { Available(AvailableEnvelope), AvailabilityPending { @@ -204,6 +224,12 @@ pub enum EnvelopeError { ExecutionPayloadError(ExecutionPayloadError), } +impl std::fmt::Display for EnvelopeError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{:?}", self) + } +} + impl From for EnvelopeError { fn from(e: BeaconChainError) -> Self { EnvelopeError::BeaconChainError(Arc::new(e)) @@ -248,7 +274,7 @@ impl From for EnvelopeError { pub(crate) fn load_snapshot( envelope: &SignedExecutionPayloadEnvelope, chain: &BeaconChain, -) -> Result, EnvelopeError> { +) -> Result, BlockError> { // Reject any block if its block is not known to fork choice. // // A block that is not in fork choice is either: @@ -263,8 +289,8 @@ pub(crate) fn load_snapshot( let fork_choice_read_lock = chain.canonical_head.fork_choice_read_lock(); let beacon_block_root = envelope.beacon_block_root(); let Some(proto_beacon_block) = fork_choice_read_lock.get_block(&beacon_block_root) else { - return Err(EnvelopeError::BlockRootUnknown { - block_root: beacon_block_root, + return Err(BlockError::ParentUnknown { + parent_root: beacon_block_root, }); }; drop(fork_choice_read_lock); @@ -278,7 +304,7 @@ pub(crate) fn load_snapshot( let state = chain .store .get_hot_state(&block_state_root, cache_state) - .map_err(|e| EnvelopeError::BeaconChainError(Arc::new(e.into())))? + .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))? .ok_or_else(|| { BeaconChainError::DBInconsistent(format!( "Missing state for envelope block {block_state_root:?}", @@ -299,11 +325,15 @@ impl IntoExecutionPendingEnvelope self, chain: &Arc>, notify_execution_layer: NotifyExecutionLayer, - ) -> Result, EnvelopeError> { + ) -> Result, BlockError> { // TODO(EIP-7732): figure out how this should be refactored.. GossipVerifiedEnvelope::new(self, chain)? .into_execution_pending_envelope(chain, notify_execution_layer) } + + fn envelope(&self) -> &Arc> { + self + } } #[derive(Clone, Debug, PartialEq)] diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs index b485af0bfa..4ac556f1f7 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs @@ -8,7 +8,7 @@ use types::{SignedBeaconBlock, SignedExecutionPayloadEnvelope}; use crate::{ BeaconChain, BeaconChainTypes, BlockError, ExecutionPayloadError, NotifyExecutionLayer, - execution_payload::notify_new_payload, + execution_payload::notify_new_payload, payload_envelope_verification::EnvelopeError, }; /// Used to await the result of executing payload with a remote EE. diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs index 381a5eaa4f..2545a900ad 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs @@ -1,10 +1,13 @@ use std::sync::Arc; -use crate::{AvailabilityProcessingStatus, BeaconChain, BeaconChainTypes, payload_envelope_verification::{ExecutedEnvelope, ExecutionPendingEnvelope}}; +use crate::{ + AvailabilityProcessingStatus, BeaconChain, BeaconChainTypes, + payload_envelope_verification::{ExecutedEnvelope, ExecutionPendingEnvelope}, +}; async fn import_execution_pending_envelope( chain: Arc>, - execution_pending_envelope: ExecutionPendingEnvelope, + execution_pending_envelope: ExecutionPendingEnvelope, ) -> Result { match chain .clone() diff --git a/beacon_node/beacon_chain/src/validator_monitor.rs b/beacon_node/beacon_chain/src/validator_monitor.rs index fdc7d27320..d8c161f8c1 100644 --- a/beacon_node/beacon_chain/src/validator_monitor.rs +++ b/beacon_node/beacon_chain/src/validator_monitor.rs @@ -30,7 +30,7 @@ use types::{ Attestation, AttestationData, AttesterSlashingRef, BeaconBlockRef, BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec, Hash256, IndexedAttestation, IndexedAttestationRef, ProposerSlashing, SignedAggregateAndProof, SignedContributionAndProof, - Slot, SyncCommitteeMessage, VoluntaryExit, + SignedExecutionPayloadEnvelope, Slot, SyncCommitteeMessage, VoluntaryExit, }; /// Used for Prometheus labels. diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index a4125f3df0..bdcdd68860 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -4,7 +4,6 @@ use crate::{ service::NetworkMessage, sync::SyncMessage, }; -use beacon_chain::blob_verification::{GossipBlobError, GossipVerifiedBlob}; use beacon_chain::block_verification_types::AsBlock; use beacon_chain::data_column_verification::{GossipDataColumnError, GossipVerifiedDataColumn}; use beacon_chain::store::Error; @@ -19,6 +18,12 @@ use beacon_chain::{ sync_committee_verification::{self, Error as SyncCommitteeError}, validator_monitor::{get_block_delay_ms, get_slot_delay_ms}, }; +use beacon_chain::{ + blob_verification::{GossipBlobError, GossipVerifiedBlob}, + payload_envelope_verification::{ + EnvelopeError, gossip_verified_envelope::GossipVerifiedEnvelope, + }, +}; use beacon_processor::{Work, WorkEvent}; use lighthouse_network::{Client, MessageAcceptance, MessageId, PeerAction, PeerId, ReportSource}; use logging::crit; @@ -3224,20 +3229,20 @@ impl NetworkBeaconProcessor { } } - pub async fn process_gossip_execution_payload( + pub async fn process_gossip_execution_payload_envelope( self: &Arc, message_id: MessageId, peer_id: PeerId, - execution_payload: SignedExecutionPayloadEnvelope, + envelope: SignedExecutionPayloadEnvelope, ) { // TODO(EIP-7732): Implement proper execution payload envelope gossip processing. // This should integrate with the envelope_verification.rs module once it's implemented. trace!( %peer_id, - builder_index = execution_payload.message.builder_index, - slot = %execution_payload.message.slot, - beacon_block_root = %execution_payload.message.beacon_block_root, + builder_index = envelope.message.builder_index, + slot = %envelope.message.slot, + beacon_block_root = %envelope.message.beacon_block_root, "Processing execution payload envelope" ); @@ -3245,6 +3250,134 @@ impl NetworkBeaconProcessor { self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore); } + async fn process_gossip_unverified_execution_payload_envelope( + self: &Arc, + message_id: MessageId, + peer_id: PeerId, + envelope: Arc>, + ) -> Option> { + // TODO(glos) add payload envelope delay metrics + + let verification_result = self + .chain + .clone() + .verify_envelope_for_gossip(envelope.clone()) + .await; + + // TODO(gloas) delay metrics and write the time that the payload was observed into + // the delay cache + + let verified_envelope = match verification_result { + Ok(verified_envelope) => { + info!( + slot = %verified_envelope.signed_envelope.slot(), + root = ?verified_envelope.signed_envelope.beacon_block_root(), + "New envelope received" + ); + + self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Accept); + + verified_envelope + } + // TODO(gloas) penalize peers accordingly + Err(_) => return None, + }; + + // TODO(gloas) do we need to register the payload with monitored validators? + + let envelope_slot = verified_envelope.signed_envelope.slot(); + let beacon_block_root = verified_envelope.signed_envelope.beacon_block_root(); + match self.chain.slot() { + // We only need to do a simple check about the envelope slot vs the current slot beacuse + // `verify_envelope_for_gossip` already ensuresthat the envelope slot is within tolerance + // for envelope imports. + Ok(current_slot) if envelope_slot > current_slot => { + warn!( + ?envelope_slot, + ?beacon_block_root, + msg = "if this happens consistently, check system clock", + "envelope arrived early" + ); + + // TODO(gloas) update metrics to note how early the envelope arrived + + let inner_self = self.clone(); + let process_fn = Box::pin(async move { + inner_self + .process_gossip_verified_execution_payload_envelope( + peer_id, + verified_envelope, + ) + .await; + }); + + // TODO(gloas) send to reprocess queue + None + } + Ok(_) => Some(verified_envelope), + Err(e) => { + error!( + error = ?e, + %envelope_slot, + ?beacon_block_root, + location = "envelope gossip", + "Failed to defer envelope import" + ); + None + } + } + } + + async fn process_gossip_verified_execution_payload_envelope( + self: Arc, + peer_id: PeerId, + verified_envelope: GossipVerifiedEnvelope, + ) { + let processing_start_time = Instant::now(); + let envelope = verified_envelope.envelope_cloned(); + let beacon_block_root = verified_envelope.signed_envelope.beacon_block_root(); + + let result = self + .chain + .process_execution_payload_envelope( + block_root, + verified_envelope, + notify_execution_layer, + block_source, + publish_fn, + ) + .await; + + register_process_result_metrics(&result, metrics::BlockSource::Gossip, "envelope"); + + match &result { + Ok(AvailabilityProcessingStatus::Imported(block_root)) => { + // TODO(gloas) do we need to send a `PayloadImported` event to the reporcess queue? + + debug!( + ?block_root, + %peer_id, + "Gossipsub envelope processed" + ); + + // TODO(gloas) do we need to recompute head? + // should canonical_head return the block and the payload now? + self.chain.recompute_head_at_current_slot().await; + + // TODO(gloas) metrics + } + Ok(AvailabilityProcessingStatus::MissingComponents(slot, block_root)) => { + trace!( + %slot, + %block_root, + "Processed envelope, waiting for other components" + ) + } + + Err(_) => todo!(), + } + } + pub fn process_gossip_execution_payload_bid( self: &Arc, message_id: MessageId, From 47782a68c31c4b4c10a597562d6a94032546e9d8 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Thu, 12 Feb 2026 21:27:39 -0800 Subject: [PATCH 06/19] delay cache, and remove some todos --- beacon_node/beacon_chain/src/beacon_chain.rs | 16 +- beacon_node/beacon_chain/src/builder.rs | 1 + .../beacon_chain/src/envelope_times_cache.rs | 197 ++++++++++++++++++ beacon_node/beacon_chain/src/lib.rs | 2 +- beacon_node/beacon_chain/src/metrics.rs | 38 ++++ .../gossip_verified_envelope.rs | 17 +- .../import.rs} | 172 ++++++++------- .../src/payload_envelope_verification/mod.rs | 58 +++--- .../payload_notifier.rs | 8 +- .../payload_envelope_verification/tests.rs | 23 -- .../beacon_chain/src/validator_monitor.rs | 2 +- .../beacon_chain/tests/block_verification.rs | 2 +- beacon_node/network/src/metrics.rs | 10 + .../gossip_methods.rs | 82 +++++--- .../src/network_beacon_processor/mod.rs | 8 +- beacon_node/network/src/router.rs | 1 + 16 files changed, 459 insertions(+), 178 deletions(-) create mode 100644 beacon_node/beacon_chain/src/envelope_times_cache.rs rename beacon_node/beacon_chain/src/{payload_envelope_import/mod.rs => payload_envelope_verification/import.rs} (68%) delete mode 100644 beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 59dd0fd1fe..4af147addb 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -27,6 +27,7 @@ use crate::data_availability_checker::{ }; use crate::data_column_verification::{GossipDataColumnError, GossipVerifiedDataColumn}; use crate::early_attester_cache::EarlyAttesterCache; +use crate::envelope_times_cache::EnvelopeTimesCache; use crate::errors::{BeaconChainError as Error, BlockProductionError}; use crate::events::ServerSentEventHandler; use crate::execution_payload::{NotifyExecutionLayer, PreparePayloadHandle, get_execution_payload}; @@ -56,7 +57,9 @@ use crate::observed_block_producers::ObservedBlockProducers; use crate::observed_data_sidecars::ObservedDataSidecars; use crate::observed_operations::{ObservationOutcome, ObservedOperations}; use crate::observed_slashable::ObservedSlashable; -use crate::payload_envelope_verification::{ExecutedEnvelope, ExecutionPendingEnvelope}; +use crate::payload_envelope_verification::{ + EnvelopeError, ExecutedEnvelope, ExecutionPendingEnvelope, +}; use crate::persisted_beacon_chain::PersistedBeaconChain; use crate::persisted_custody::persist_custody_context; use crate::persisted_fork_choice::PersistedForkChoice; @@ -460,6 +463,8 @@ pub struct BeaconChain { pub early_attester_cache: EarlyAttesterCache, /// A cache used to keep track of various block timings. pub block_times_cache: Arc>, + /// A cache used to keep track of various envelope timings. + pub envelope_times_cache: Arc>, /// A cache used to track pre-finalization block roots for quick rejection. pub pre_finalization_block_cache: PreFinalizationBlockCache, /// A cache used to produce light_client server messages @@ -1158,8 +1163,8 @@ impl BeaconChain { match self.store.try_get_full_block(block_root)? { Some(DatabaseBlock::Full(block)) => Ok(Some(block)), Some(DatabaseBlock::Blinded(_)) => { - // TODO(gloas) this should error out - todo!() + // TODO(gloas) should we return None here? + Ok(None) } None => Ok(None), } @@ -3556,7 +3561,7 @@ impl BeaconChain { pub async fn into_executed_payload_envelope( self: Arc, pending_envelope: ExecutionPendingEnvelope, - ) -> Result, BlockError> { + ) -> Result, EnvelopeError> { let ExecutionPendingEnvelope { signed_envelope, import_data, @@ -4168,7 +4173,7 @@ impl BeaconChain { Ok(block_root) } - pub(crate) fn handle_import_block_db_write_error( + fn handle_import_block_db_write_error( &self, // We don't actually need this value, however it's always present when we call this function // and it needs to be dropped to prevent a dead-lock. Requiring it to be passed here is @@ -6698,6 +6703,7 @@ impl BeaconChain { // sync anyway). self.naive_aggregation_pool.write().prune(slot); self.block_times_cache.write().prune(slot); + self.envelope_times_cache.write().prune(slot); // Don't run heavy-weight tasks during sync. if self.best_slot() + MAX_PER_SLOT_FORK_CHOICE_DISTANCE < slot { diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index f673519f5f..3a912016dc 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -1055,6 +1055,7 @@ where )), beacon_proposer_cache, block_times_cache: <_>::default(), + envelope_times_cache: <_>::default(), pre_finalization_block_cache: <_>::default(), validator_pubkey_cache: RwLock::new(validator_pubkey_cache), early_attester_cache: <_>::default(), diff --git a/beacon_node/beacon_chain/src/envelope_times_cache.rs b/beacon_node/beacon_chain/src/envelope_times_cache.rs new file mode 100644 index 0000000000..84c936c210 --- /dev/null +++ b/beacon_node/beacon_chain/src/envelope_times_cache.rs @@ -0,0 +1,197 @@ +//! This module provides the `EnvelopeTimesCache` which contains information regarding payload +//! envelope timings. +//! +//! This provides `BeaconChain` and associated functions with access to the timestamps of when a +//! payload envelope was observed, verified, executed, and imported. +//! This allows for better traceability and allows us to determine the root cause for why an +//! envelope was imported late. +//! This allows us to distinguish between the following scenarios: +//! - The envelope was observed late. +//! - Consensus verification was slow. +//! - Execution verification was slow. +//! - The DB write was slow. + +use eth2::types::{Hash256, Slot}; +use std::collections::HashMap; +use std::time::Duration; + +type BlockRoot = Hash256; + +#[derive(Clone, Default)] +pub struct EnvelopeTimestamps { + /// When the envelope was first observed (gossip or RPC). + pub observed: Option, + /// When consensus verification (state transition) completed. + pub consensus_verified: Option, + /// When execution layer verification started. + pub started_execution: Option, + /// When execution layer verification completed. + pub executed: Option, + /// When the envelope was imported into the DB. + pub imported: Option, +} + +/// Delay data for envelope processing, computed relative to the slot start time. +#[derive(Debug, Default)] +pub struct EnvelopeDelays { + /// Time after start of slot we saw the envelope. + pub observed: Option, + /// The time it took to complete consensus verification of the envelope. + pub consensus_verification_time: Option, + /// The time it took to complete execution verification of the envelope. + pub execution_time: Option, + /// Time after execution until the envelope was imported. + pub imported: Option, +} + +impl EnvelopeDelays { + fn new(times: EnvelopeTimestamps, slot_start_time: Duration) -> EnvelopeDelays { + let observed = times + .observed + .and_then(|observed_time| observed_time.checked_sub(slot_start_time)); + let consensus_verification_time = times + .consensus_verified + .and_then(|consensus_verified| consensus_verified.checked_sub(times.observed?)); + let execution_time = times + .executed + .and_then(|executed| executed.checked_sub(times.started_execution?)); + let imported = times + .imported + .and_then(|imported_time| imported_time.checked_sub(times.executed?)); + EnvelopeDelays { + observed, + consensus_verification_time, + execution_time, + imported, + } + } +} + +pub struct EnvelopeTimesCacheValue { + pub slot: Slot, + pub timestamps: EnvelopeTimestamps, + pub peer_id: Option, +} + +impl EnvelopeTimesCacheValue { + fn new(slot: Slot) -> Self { + EnvelopeTimesCacheValue { + slot, + timestamps: Default::default(), + peer_id: None, + } + } +} + +#[derive(Default)] +pub struct EnvelopeTimesCache { + pub cache: HashMap, +} + +impl EnvelopeTimesCache { + /// Set the observation time for `block_root` to `timestamp` if `timestamp` is less than + /// any previous timestamp at which this envelope was observed. + pub fn set_time_observed( + &mut self, + block_root: BlockRoot, + slot: Slot, + timestamp: Duration, + peer_id: Option, + ) { + let entry = self + .cache + .entry(block_root) + .or_insert_with(|| EnvelopeTimesCacheValue::new(slot)); + match entry.timestamps.observed { + Some(existing) if existing <= timestamp => { + // Existing timestamp is earlier, do nothing. + } + _ => { + entry.timestamps.observed = Some(timestamp); + entry.peer_id = peer_id; + } + } + } + + /// Set the timestamp for `field` if that timestamp is less than any previously known value. + fn set_time_if_less( + &mut self, + block_root: BlockRoot, + slot: Slot, + field: impl Fn(&mut EnvelopeTimestamps) -> &mut Option, + timestamp: Duration, + ) { + let entry = self + .cache + .entry(block_root) + .or_insert_with(|| EnvelopeTimesCacheValue::new(slot)); + let existing_timestamp = field(&mut entry.timestamps); + if existing_timestamp.is_none_or(|prev| timestamp < prev) { + *existing_timestamp = Some(timestamp); + } + } + + pub fn set_time_consensus_verified( + &mut self, + block_root: BlockRoot, + slot: Slot, + timestamp: Duration, + ) { + self.set_time_if_less( + block_root, + slot, + |timestamps| &mut timestamps.consensus_verified, + timestamp, + ) + } + + pub fn set_time_started_execution( + &mut self, + block_root: BlockRoot, + slot: Slot, + timestamp: Duration, + ) { + self.set_time_if_less( + block_root, + slot, + |timestamps| &mut timestamps.started_execution, + timestamp, + ) + } + + pub fn set_time_executed(&mut self, block_root: BlockRoot, slot: Slot, timestamp: Duration) { + self.set_time_if_less( + block_root, + slot, + |timestamps| &mut timestamps.executed, + timestamp, + ) + } + + pub fn set_time_imported(&mut self, block_root: BlockRoot, slot: Slot, timestamp: Duration) { + self.set_time_if_less( + block_root, + slot, + |timestamps| &mut timestamps.imported, + timestamp, + ) + } + + pub fn get_envelope_delays( + &self, + block_root: BlockRoot, + slot_start_time: Duration, + ) -> EnvelopeDelays { + if let Some(entry) = self.cache.get(&block_root) { + EnvelopeDelays::new(entry.timestamps.clone(), slot_start_time) + } else { + EnvelopeDelays::default() + } + } + + /// Prune the cache to only store the most recent 2 epochs. + pub fn prune(&mut self, current_slot: Slot) { + self.cache + .retain(|_, entry| entry.slot > current_slot.saturating_sub(64_u64)); + } +} diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index f0151eed01..5acc3edadd 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -20,6 +20,7 @@ pub mod custody_context; pub mod data_availability_checker; pub mod data_column_verification; mod early_attester_cache; +pub mod envelope_times_cache; mod errors; pub mod events; pub mod execution_payload; @@ -42,7 +43,6 @@ pub mod observed_block_producers; pub mod observed_data_sidecars; pub mod observed_operations; mod observed_slashable; -pub mod payload_envelope_import; pub mod payload_envelope_verification; pub mod persisted_beacon_chain; pub mod persisted_custody; diff --git a/beacon_node/beacon_chain/src/metrics.rs b/beacon_node/beacon_chain/src/metrics.rs index 9de67ca93f..775f5a3df0 100644 --- a/beacon_node/beacon_chain/src/metrics.rs +++ b/beacon_node/beacon_chain/src/metrics.rs @@ -21,6 +21,44 @@ pub const VALIDATOR_MONITOR_ATTESTATION_SIMULATOR_SOURCE_ATTESTER_HIT_TOTAL: &st pub const VALIDATOR_MONITOR_ATTESTATION_SIMULATOR_SOURCE_ATTESTER_MISS_TOTAL: &str = "validator_monitor_attestation_simulator_source_attester_miss_total"; +/* +* Execution Payload Envelope Procsesing +*/ + +pub static ENVELOPE_PROCESSING_REQUESTS: LazyLock> = LazyLock::new(|| { + try_create_int_counter( + "payload_envelope_processing_requests_total", + "Count of payload envelopes submitted for processing", + ) +}); +pub static ENVELOPE_PROCESSING_SUCCESSES: LazyLock> = LazyLock::new(|| { + try_create_int_counter( + "payload_envelope_processing_successes_total", + "Count of payload envelopes processed without error", + ) +}); +pub static ENVELOPE_PROCESSING_TIMES: LazyLock> = LazyLock::new(|| { + try_create_histogram( + "payload_envelope_processing_seconds", + "Full runtime of payload envelope processing", + ) +}); +pub static ENVELOPE_PROCESSING_DB_WRITE: LazyLock> = LazyLock::new(|| { + try_create_histogram( + "payload_envelope_processing_db_write_seconds", + "Time spent writing a newly processed payload envelope and state to DB", + ) +}); +pub static ENVELOPE_PROCESSING_POST_EXEC_PROCESSING: LazyLock> = + LazyLock::new(|| { + try_create_histogram_with_buckets( + "payload_envelope_processing_post_exec_pre_attestable_seconds", + "Time between finishing execution processing and the payload envelope + becoming attestable", + linear_buckets(0.01, 0.01, 15), + ) + }); + /* * Block Processing */ diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs index 5af6fe3984..c9bef630aa 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs @@ -10,7 +10,7 @@ use types::{ }; use crate::{ - BeaconChain, BeaconChainError, BeaconChainTypes, BlockError, NotifyExecutionLayer, + BeaconChain, BeaconChainError, BeaconChainTypes, NotifyExecutionLayer, PayloadVerificationOutcome, payload_envelope_verification::{ EnvelopeError, EnvelopeImportData, EnvelopeProcessingSnapshot, ExecutionPendingEnvelope, @@ -151,8 +151,7 @@ impl GossipVerifiedEnvelope { ); (is_valid, opt_snapshot) } else { - // TODO(gloas) we should probably introduce a builder cache or some type of - // global cache. + // TODO(gloas) if we implement a builder pubkey cache, we'll need to use it here. // External builder: must load the state to get the builder pubkey. let snapshot = load_snapshot(signed_envelope.as_ref(), chain)?; let is_valid = @@ -181,7 +180,7 @@ impl IntoExecutionPendingEnvelope for GossipVerifiedEnve self, chain: &Arc>, notify_execution_layer: NotifyExecutionLayer, - ) -> Result, BlockError> { + ) -> Result, EnvelopeError> { let signed_envelope = self.signed_envelope; let envelope = &signed_envelope.message; let payload = &envelope.payload; @@ -198,13 +197,11 @@ impl IntoExecutionPendingEnvelope for GossipVerifiedEnve let payload_verification_future = async move { let chain = payload_notifier.chain.clone(); - // TODO:(gloas): timing metrics if let Some(started_execution) = chain.slot_clock.now_duration() { - chain.block_times_cache.write().set_time_started_execution( - block_root, - slot, - started_execution, - ); + chain + .envelope_times_cache + .write() + .set_time_started_execution(block_root, slot, started_execution); } let payload_verification_status = payload_notifier.notify_new_payload().await?; diff --git a/beacon_node/beacon_chain/src/payload_envelope_import/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs similarity index 68% rename from beacon_node/beacon_chain/src/payload_envelope_import/mod.rs rename to beacon_node/beacon_chain/src/payload_envelope_verification/import.rs index ab7de5c15d..3628712721 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_import/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -1,20 +1,23 @@ use std::sync::Arc; +use std::time::Duration; use fork_choice::PayloadVerificationStatus; use logging::crit; +use slot_clock::SlotClock; use store::StoreOp; use tracing::{debug, error, info_span, instrument}; -use types::{BeaconState, BlockImportSource, EthSpec, Hash256, SignedBeaconBlock}; +use types::{BeaconState, BlockImportSource, Hash256, SignedBeaconBlock, Slot}; +use super::{ + AvailableEnvelope, AvailableExecutedEnvelope, EnvelopeError, EnvelopeImportData, + ExecutedEnvelope, IntoExecutionPendingEnvelope, +}; use crate::{ - AvailabilityProcessingStatus, BeaconChain, BeaconChainError, BeaconChainTypes, BlockError, + AvailabilityProcessingStatus, BeaconChain, BeaconChainError, BeaconChainTypes, NotifyExecutionLayer, block_verification_types::{AsBlock, AvailableBlockData}, - payload_envelope_verification::{ - AvailableEnvelope, AvailableExecutedEnvelope, EnvelopeImportData, ExecutedEnvelope, - IntoExecutionPendingEnvelope, - }, - validator_monitor::timestamp_now, + metrics, + validator_monitor::{get_slot_delay_ms, timestamp_now}, }; impl BeaconChain { @@ -36,21 +39,26 @@ impl BeaconChain { unverified_envelope: P, notify_execution_layer: NotifyExecutionLayer, block_source: BlockImportSource, - publish_fn: impl FnOnce() -> Result<(), BlockError>, - ) -> Result { + publish_fn: impl FnOnce() -> Result<(), EnvelopeError>, + ) -> Result { let block_slot = unverified_envelope.envelope().slot(); - // TODO(gloas) Set observed time if not already set. Usually this should be set by gossip or RPC, + // Set observed time if not already set. Usually this should be set by gossip or RPC, // but just in case we set it again here (useful for tests). + if let Some(seen_timestamp) = self.slot_clock.now_duration() { + self.envelope_times_cache.write().set_time_observed( + block_root, + block_slot, + seen_timestamp, + None, + ); + } - // TODO(gloas) if we decide to insert the payload envelope into the new DA checker - // we should insert the pre executed envelope here. + // TODO(gloas) insert the pre-executed envelope into some type of cache. - // TODO(gloas) Start the Prometheus timer. - // let _full_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_TIMES); + let _full_timer = metrics::start_timer(&metrics::ENVELOPE_PROCESSING_TIMES); - // TODO(gloas) Increment the Prometheus counter for envelope processing requests. - // metrics::inc_counter(&metrics::BLOCK_PROCESSING_REQUESTS); + metrics::inc_counter(&metrics::ENVELOPE_PROCESSING_REQUESTS); // A small closure to group the verification and import errors. let chain = self.clone(); @@ -59,12 +67,16 @@ impl BeaconChain { .into_execution_pending_envelope(&chain, notify_execution_layer)?; publish_fn()?; - // TODO(gloas) Record the time it took to complete consensus verification. - // if let Some(timestamp) = self.slot_clock.now_duration() { - // self.block_times_cache - // .write() - // .set_time_consensus_verified(block_root, block_slot, timestamp) - // } + // Record the time it took to complete consensus verification. + if let Some(timestamp) = chain.slot_clock.now_duration() { + chain + .envelope_times_cache + .write() + .set_time_consensus_verified(block_root, block_slot, timestamp); + } + + let envelope_times_cache = chain.envelope_times_cache.clone(); + let slot_clock = chain.slot_clock.clone(); let executed_envelope = chain .into_executed_payload_envelope(execution_pending) @@ -75,28 +87,23 @@ impl BeaconChain { // reprocess this block until the block is evicted from DA checker, causing the // chain to get stuck temporarily if the block is canonical. Therefore we remove // it from the cache if execution fails. - - //self.data_availability_checker - // .remove_block_on_execution_error(&block_root); })?; - // TODO(gloas) Record the *additional* time it took to wait for execution layer verification. - // if let Some(timestamp) = self.slot_clock.now_duration() { - // self.block_times_cache - // .write() - // .set_time_executed(block_root, block_slot, timestamp) - // } + // Record the time it took to wait for execution layer verification. + if let Some(timestamp) = slot_clock.now_duration() { + envelope_times_cache + .write() + .set_time_executed(block_root, block_slot, timestamp); + } match executed_envelope { ExecutedEnvelope::Available(envelope) => { self.import_available_execution_payload_envelope(Box::new(envelope)) .await } - ExecutedEnvelope::AvailabilityPending() => { - return Err(BlockError::InternalError( - "Pending payload envelope not yet implemented".to_owned(), - )); - } + ExecutedEnvelope::AvailabilityPending() => Err(EnvelopeError::InternalError( + "Pending payload envelope not yet implemented".to_owned(), + )), } }; @@ -111,8 +118,7 @@ impl BeaconChain { "Envelope imported" ); - // TODO(gloas) Increment the Prometheus counter for block processing successes. - // metrics::inc_counter(&metrics::BLOCK_PROCESSING_SUCCESSES); + metrics::inc_counter(&metrics::ENVELOPE_PROCESSING_SUCCESSES); Ok(status) } @@ -121,7 +127,7 @@ impl BeaconChain { Ok(status) } - Err(BlockError::BeaconChainError(e)) => { + Err(EnvelopeError::BeaconChainError(e)) => { match e.as_ref() { BeaconChainError::TokioJoin(e) => { debug!( @@ -138,7 +144,7 @@ impl BeaconChain { ); } }; - Err(BlockError::BeaconChainError(e)) + Err(EnvelopeError::BeaconChainError(e)) } // The block failed verification. Err(other) => { @@ -152,7 +158,7 @@ impl BeaconChain { pub async fn import_available_execution_payload_envelope( self: &Arc, envelope: Box>, - ) -> Result { + ) -> Result { let AvailableExecutedEnvelope { envelope, import_data, @@ -165,8 +171,6 @@ impl BeaconChain { post_state, } = import_data; - // TODO(gloas) Record the time at which this block's blobs became available. - let block_root = { // Capture the current span before moving into the blocking task let current_span = tracing::Span::current(); @@ -202,21 +206,18 @@ impl BeaconChain { &self, signed_envelope: AvailableEnvelope, block_root: Hash256, - mut state: BeaconState, - payload_verification_status: PayloadVerificationStatus, + state: BeaconState, + _payload_verification_status: PayloadVerificationStatus, parent_block: Arc>, - ) -> Result { + ) -> Result { // ----------------------------- ENVELOPE NOT YET ATTESTABLE ---------------------------------- // Everything in this initial section is on the hot path between processing the envelope and // being able to attest to it. DO NOT add any extra processing in this initial section // unless it must run before fork choice. // ----------------------------------------------------------------------------------------- - let current_slot = self.slot()?; - let current_epoch = current_slot.epoch(T::EthSpec::slots_per_epoch()); - let envelope = signed_envelope.message(); - // TODO(gloas) implement metrics - // let post_exec_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_POST_EXEC_PROCESSING); + let post_exec_timer = + metrics::start_timer(&metrics::ENVELOPE_PROCESSING_POST_EXEC_PROCESSING); // Check the payloads parent block against weak subjectivity checkpoint. self.check_block_against_weak_subjectivity_checkpoint( @@ -228,26 +229,24 @@ impl BeaconChain { // Take an upgradable read lock on fork choice so we can check if this block has already // been imported. We don't want to repeat work importing a block that is already imported. let fork_choice_reader = self.canonical_head.fork_choice_upgradable_read_lock(); - if fork_choice_reader.contains_block(&block_root) { - return Err(BlockError::DuplicateFullyImported(block_root)); + if !fork_choice_reader.contains_block(&block_root) { + return Err(EnvelopeError::BlockRootUnknown { block_root }); } + // TODO(gloas) no fork choice logic yet // Take an exclusive write-lock on fork choice. It's very important to prevent deadlocks by // avoiding taking other locks whilst holding this lock. - let mut fork_choice = parking_lot::RwLockUpgradableReadGuard::upgrade(fork_choice_reader); + // let fork_choice = parking_lot::RwLockUpgradableReadGuard::upgrade(fork_choice_reader); // TODO(gloas) Do we need this check? Do not import a block that doesn't descend from the finalized root. // let signed_block = check_block_is_finalized_checkpoint_or_descendant(self, &fork_choice, signed_block)?; - // TODO(gloas)Do we want to use an early attester cache like mechanism for payload enevelopes? + // TODO(gloas) Do we want to use an early attester cache like mechanism for payload enevelopes? // TODO(gloas) emit SSE event if the payload became the new head payload - // drop(post_exec_timer); + drop(post_exec_timer); - // ---------------------------- BLOCK PROBABLY ATTESTABLE ---------------------------------- - // Most blocks are now capable of being attested to thanks to the `early_attester_cache` - // cache above. Resume non-essential processing. - // - // It is important NOT to return errors here before the database commit, because the block + // ---------------------------- ENVELOPE PROBABLY ATTESTABLE ---------------------------------- + // It is important NOT to return errors here before the database commit, because the envelope // has already been added to fork choice and the database would be left in an inconsistent // state if we returned early without committing. In other words, an error here would // corrupt the node's database permanently. @@ -278,15 +277,13 @@ impl BeaconChain { ?block_root, "Failed to store data columns into the database" ); - return Err(self - .handle_import_block_db_write_error(fork_choice) - .err() - .unwrap_or(BlockError::InternalError(e))); + // TODO(gloas) implement failed write handling to fork choice + // let _ = self.handle_import_block_db_write_error(fork_choice); + return Err(EnvelopeError::InternalError(e)); } } - // TODO(gloas) metrics - // let db_write_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_DB_WRITE); + let db_write_timer = metrics::start_timer(&metrics::ENVELOPE_PROCESSING_DB_WRITE); ops.push(StoreOp::PutPayloadEnvelope( block_root, @@ -299,7 +296,6 @@ impl BeaconChain { let db_span = info_span!("persist_payloads_and_blobs").entered(); - // TODO(gloas) do i need this if let Err(e) = self.store.do_atomically_with_block_and_blobs_cache(ops) { error!( msg = "Restoring fork choice from disk", @@ -315,25 +311,49 @@ impl BeaconChain { drop(db_span); + // TODO(gloas) drop fork choice lock // The fork choice write-lock is dropped *after* the on-disk database has been updated. // This prevents inconsistency between the two at the expense of concurrency. - drop(fork_choice); + // drop(fork_choice); // We're declaring the envelope "imported" at this point, since fork choice and the DB know // about it. let envelope_time_imported = timestamp_now(); // TODO(gloas) depending on what happens with light clients - // we might need to do some computations here + // we might need to do some light client related computations here - // TODO(gloas) metrics - // metrics::stop_timer(db_write_timer); + metrics::stop_timer(db_write_timer); + metrics::inc_counter(&metrics::ENVELOPE_PROCESSING_SUCCESSES); - // TODO(gloas) metrics - // metrics::inc_counter(&metrics::BLOCK_PROCESSING_SUCCESSES); + self.import_envelope_update_metrics_and_events( + block_root, + signed_envelope.slot(), + envelope_time_imported, + ); - // TODO(gloas) we might want to implement something similar - // to `import_block_update_metrics_and_events` Ok(block_root) } + + fn import_envelope_update_metrics_and_events( + &self, + block_root: Hash256, + envelope_slot: Slot, + envelope_time_imported: Duration, + ) { + let envelope_delay_total = + get_slot_delay_ms(envelope_time_imported, envelope_slot, &self.slot_clock); + + // Do not write to the cache for envelopes older than 2 epochs, this helps reduce writes + // to the cache during sync. + if envelope_delay_total < self.slot_clock.slot_duration().saturating_mul(64) { + self.envelope_times_cache.write().set_time_imported( + block_root, + envelope_slot, + envelope_time_imported, + ); + } + + // TODO(gloas) emit SSE event for envelope import (similar to SseBlock for blocks). + } } diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs index 4dd8d351d7..80e62f93b7 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs @@ -1,8 +1,9 @@ //! The incremental processing steps (e.g., signatures verified but not the state transition) is //! represented as a sequence of wrapper-types around the block. There is a linear progression of -//! types, starting at a `SignedBeaconBlock` and finishing with a `Fully VerifiedBlock` (see +//! types, starting at a `SignedExecutionPayloadEnvelope` and finishing with an `AvailableExecutedEnvelope` (see //! diagram below). //! +//! // TODO(gloas) we might want to update this diagram to include `AvailabelExecutedEnvelope` //! ```ignore //! START //! | @@ -28,9 +29,9 @@ use std::sync::Arc; -use state_processing::{ - BlockProcessingError, ConsensusContext, envelope_processing::EnvelopeProcessingError, -}; +use store::Error as DBError; + +use state_processing::{BlockProcessingError, envelope_processing::EnvelopeProcessingError}; use tracing::instrument; use types::{ BeaconState, BeaconStateError, ChainSpec, DataColumnSidecarList, EthSpec, ExecutionBlockHash, @@ -45,15 +46,15 @@ use crate::{ }; pub mod gossip_verified_envelope; +pub mod import; mod payload_notifier; -mod tests; pub trait IntoExecutionPendingEnvelope: Sized { fn into_execution_pending_envelope( self, chain: &Arc>, notify_execution_layer: NotifyExecutionLayer, - ) -> Result, BlockError>; + ) -> Result, EnvelopeError>; fn envelope(&self) -> &Arc>; } @@ -74,8 +75,7 @@ pub struct EnvelopeImportData { #[derive(Debug)] #[allow(dead_code)] pub struct AvailableEnvelope { - // TODO(EIP-7732): rename to execution_block_hash - block_hash: ExecutionBlockHash, + execution_block_hash: ExecutionBlockHash, envelope: Arc>, columns: DataColumnSidecarList, /// Timestamp at which this block first became available (UNIX timestamp, time since 1970). @@ -222,6 +222,10 @@ pub enum EnvelopeError { EnvelopeProcessingError(EnvelopeProcessingError), // Error verifying the execution payload ExecutionPayloadError(ExecutionPayloadError), + // An error from block-level checks reused during envelope import + BlockError(BlockError), + // Internal error + InternalError(String), } impl std::fmt::Display for EnvelopeError { @@ -248,6 +252,18 @@ impl From for EnvelopeError { } } +impl From for EnvelopeError { + fn from(e: DBError) -> Self { + EnvelopeError::BeaconChainError(Arc::new(BeaconChainError::DBError(e))) + } +} + +impl From for EnvelopeError { + fn from(e: BlockError) -> Self { + EnvelopeError::BlockError(e) + } +} + /// Pull errors up from EnvelopeProcessingError to EnvelopeError impl From for EnvelopeError { fn from(e: EnvelopeProcessingError) -> Self { @@ -274,14 +290,14 @@ impl From for EnvelopeError { pub(crate) fn load_snapshot( envelope: &SignedExecutionPayloadEnvelope, chain: &BeaconChain, -) -> Result, BlockError> { - // Reject any block if its block is not known to fork choice. +) -> Result, EnvelopeError> { + // Reject any envelope if its block is not known to fork choice. // // A block that is not in fork choice is either: // - // - Not yet imported: we should reject this block because we should only import a child - // envelope after its parent has been fully imported. - // - Pre-finalized: if the block is _prior_ to finalization, we should ignore the envelope + // - Not yet imported: we should reject this envelope because we should only import it after its parent block + // has been fully imported. + // - Pre-finalized: if the parent block is _prior_ to finalization, we should ignore the envelope // because it will revert finalization. Note that the finalized block is stored in fork // choice, so we will not reject any child of the finalized block (this is relevant during // genesis). @@ -289,8 +305,8 @@ pub(crate) fn load_snapshot( let fork_choice_read_lock = chain.canonical_head.fork_choice_read_lock(); let beacon_block_root = envelope.beacon_block_root(); let Some(proto_beacon_block) = fork_choice_read_lock.get_block(&beacon_block_root) else { - return Err(BlockError::ParentUnknown { - parent_root: beacon_block_root, + return Err(EnvelopeError::BlockRootUnknown { + block_root: beacon_block_root, }); }; drop(fork_choice_read_lock); @@ -304,7 +320,7 @@ pub(crate) fn load_snapshot( let state = chain .store .get_hot_state(&block_state_root, cache_state) - .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))? + .map_err(EnvelopeError::from)? .ok_or_else(|| { BeaconChainError::DBInconsistent(format!( "Missing state for envelope block {block_state_root:?}", @@ -325,8 +341,7 @@ impl IntoExecutionPendingEnvelope self, chain: &Arc>, notify_execution_layer: NotifyExecutionLayer, - ) -> Result, BlockError> { - // TODO(EIP-7732): figure out how this should be refactored.. + ) -> Result, EnvelopeError> { GossipVerifiedEnvelope::new(self, chain)? .into_execution_pending_envelope(chain, notify_execution_layer) } @@ -335,10 +350,3 @@ impl IntoExecutionPendingEnvelope self } } - -#[derive(Clone, Debug, PartialEq)] -pub struct PayloadEnvelopeImportData { - pub block_root: Hash256, - pub state: BeaconState, - pub consensus_context: ConsensusContext, -} diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs index 4ac556f1f7..5b1f332b5a 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs @@ -7,7 +7,7 @@ use tracing::warn; use types::{SignedBeaconBlock, SignedExecutionPayloadEnvelope}; use crate::{ - BeaconChain, BeaconChainTypes, BlockError, ExecutionPayloadError, NotifyExecutionLayer, + BeaconChain, BeaconChainTypes, BlockError, NotifyExecutionLayer, execution_payload::notify_new_payload, payload_envelope_verification::EnvelopeError, }; @@ -25,15 +25,13 @@ impl PayloadNotifier { envelope: Arc>, block: Arc>, notify_execution_layer: NotifyExecutionLayer, - ) -> Result { + ) -> Result { let payload_verification_status = { let payload_message = &envelope.message; match notify_execution_layer { NotifyExecutionLayer::No if chain.config.optimistic_finalized_sync => { - // TODO(gloas) unwrap - let new_payload_request = - Self::build_new_payload_request(&envelope, &block).unwrap(); + let new_payload_request = Self::build_new_payload_request(&envelope, &block)?; if let Err(e) = new_payload_request.perform_optimistic_sync_verifications() { warn!( block_number = ?payload_message.payload.block_number, diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs deleted file mode 100644 index 2545a900ad..0000000000 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs +++ /dev/null @@ -1,23 +0,0 @@ -use std::sync::Arc; - -use crate::{ - AvailabilityProcessingStatus, BeaconChain, BeaconChainTypes, - payload_envelope_verification::{ExecutedEnvelope, ExecutionPendingEnvelope}, -}; - -async fn import_execution_pending_envelope( - chain: Arc>, - execution_pending_envelope: ExecutionPendingEnvelope, -) -> Result { - match chain - .clone() - .into_executed_payload_envelope(execution_pending_envelope) - .await - .unwrap() - { - ExecutedEnvelope::Available(envelope) => todo!(), - ExecutedEnvelope::AvailabilityPending() => { - Err("AvailabilityPending not expected in this test. Block not imported.".to_string()) - } - } -} diff --git a/beacon_node/beacon_chain/src/validator_monitor.rs b/beacon_node/beacon_chain/src/validator_monitor.rs index d8c161f8c1..fdc7d27320 100644 --- a/beacon_node/beacon_chain/src/validator_monitor.rs +++ b/beacon_node/beacon_chain/src/validator_monitor.rs @@ -30,7 +30,7 @@ use types::{ Attestation, AttestationData, AttesterSlashingRef, BeaconBlockRef, BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec, Hash256, IndexedAttestation, IndexedAttestationRef, ProposerSlashing, SignedAggregateAndProof, SignedContributionAndProof, - SignedExecutionPayloadEnvelope, Slot, SyncCommitteeMessage, VoluntaryExit, + Slot, SyncCommitteeMessage, VoluntaryExit, }; /// Used for Prometheus labels. diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index d214ea6b15..779e196495 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -1,5 +1,5 @@ #![cfg(not(debug_assertions))] - +// TODO(gloas) we probably need similar test for payload envelope verification use beacon_chain::block_verification_types::{AsBlock, ExecutedBlock, RpcBlock}; use beacon_chain::data_availability_checker::{AvailabilityCheckError, AvailableBlockData}; use beacon_chain::data_column_verification::CustodyDataColumn; diff --git a/beacon_node/network/src/metrics.rs b/beacon_node/network/src/metrics.rs index 0016f66c01..098d6d8324 100644 --- a/beacon_node/network/src/metrics.rs +++ b/beacon_node/network/src/metrics.rs @@ -532,6 +532,16 @@ pub static SYNC_RPC_REQUEST_TIME: LazyLock> = LazyLock::new ) }); +/* + * Execution Payload Envelope Delay Metrics + */ +pub static ENVELOPE_DELAY_GOSSIP: LazyLock> = LazyLock::new(|| { + try_create_int_gauge( + "payload_envelope_delay_gossip", + "The first time we see this payload envelope from gossip as a delay from the start of the slot", + ) +}); + /* * Block Delay Metrics */ diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index bdcdd68860..1dcfea96d9 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -20,9 +20,7 @@ use beacon_chain::{ }; use beacon_chain::{ blob_verification::{GossipBlobError, GossipVerifiedBlob}, - payload_envelope_verification::{ - EnvelopeError, gossip_verified_envelope::GossipVerifiedEnvelope, - }, + payload_envelope_verification::gossip_verified_envelope::GossipVerifiedEnvelope, }; use beacon_processor::{Work, WorkEvent}; use lighthouse_network::{Client, MessageAcceptance, MessageId, PeerAction, PeerId, ReportSource}; @@ -3230,24 +3228,34 @@ impl NetworkBeaconProcessor { } pub async fn process_gossip_execution_payload_envelope( - self: &Arc, + self: Arc, message_id: MessageId, peer_id: PeerId, - envelope: SignedExecutionPayloadEnvelope, + envelope: Arc>, + seen_timestamp: Duration, ) { - // TODO(EIP-7732): Implement proper execution payload envelope gossip processing. - // This should integrate with the envelope_verification.rs module once it's implemented. + if let Some(gossip_verified_envelope) = self + .process_gossip_unverified_execution_payload_envelope( + message_id, + peer_id, + envelope.clone(), + seen_timestamp, + ) + .await + { + let beacon_block_root = gossip_verified_envelope.signed_envelope.beacon_block_root(); - trace!( - %peer_id, - builder_index = envelope.message.builder_index, - slot = %envelope.message.slot, - beacon_block_root = %envelope.message.beacon_block_root, - "Processing execution payload envelope" - ); + Span::current().record("beacon_block_root", beacon_block_root.to_string()); - // For now, ignore all envelopes since verification is not implemented - self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore); + // TODO(gloas) in process_gossip_block here we check_and_insert on the duplicate cache + // before calling gossip_verified_block + + self.process_gossip_verified_execution_payload_envelope( + peer_id, + gossip_verified_envelope, + ) + .await; + } } async fn process_gossip_unverified_execution_payload_envelope( @@ -3255,8 +3263,10 @@ impl NetworkBeaconProcessor { message_id: MessageId, peer_id: PeerId, envelope: Arc>, + seen_duration: Duration, ) -> Option> { - // TODO(glos) add payload envelope delay metrics + let envelope_delay = + get_slot_delay_ms(seen_duration, envelope.slot(), &self.chain.slot_clock); let verification_result = self .chain @@ -3264,11 +3274,21 @@ impl NetworkBeaconProcessor { .verify_envelope_for_gossip(envelope.clone()) .await; - // TODO(gloas) delay metrics and write the time that the payload was observed into - // the delay cache - let verified_envelope = match verification_result { Ok(verified_envelope) => { + metrics::set_gauge( + &metrics::ENVELOPE_DELAY_GOSSIP, + envelope_delay.as_millis() as i64, + ); + + // Write the time the envelope was observed into the delay cache. + self.chain.envelope_times_cache.write().set_time_observed( + verified_envelope.signed_envelope.beacon_block_root(), + envelope.slot(), + seen_duration, + Some(peer_id.to_string()), + ); + info!( slot = %verified_envelope.signed_envelope.slot(), root = ?verified_envelope.signed_envelope.beacon_block_root(), @@ -3302,7 +3322,7 @@ impl NetworkBeaconProcessor { // TODO(gloas) update metrics to note how early the envelope arrived let inner_self = self.clone(); - let process_fn = Box::pin(async move { + let _process_fn = Box::pin(async move { inner_self .process_gossip_verified_execution_payload_envelope( peer_id, @@ -3333,27 +3353,26 @@ impl NetworkBeaconProcessor { peer_id: PeerId, verified_envelope: GossipVerifiedEnvelope, ) { - let processing_start_time = Instant::now(); - let envelope = verified_envelope.envelope_cloned(); + let _processing_start_time = Instant::now(); let beacon_block_root = verified_envelope.signed_envelope.beacon_block_root(); let result = self .chain .process_execution_payload_envelope( - block_root, + beacon_block_root, verified_envelope, - notify_execution_layer, - block_source, - publish_fn, + NotifyExecutionLayer::Yes, + BlockImportSource::Gossip, + || Ok(()), ) .await; - register_process_result_metrics(&result, metrics::BlockSource::Gossip, "envelope"); + // TODO(gloas) metrics + // register_process_result_metrics(&result, metrics::BlockSource::Gossip, "envelope"); match &result { Ok(AvailabilityProcessingStatus::Imported(block_root)) => { // TODO(gloas) do we need to send a `PayloadImported` event to the reporcess queue? - debug!( ?block_root, %peer_id, @@ -3374,7 +3393,10 @@ impl NetworkBeaconProcessor { ) } - Err(_) => todo!(), + Err(_) => { + // TODO(gloas) implement peer penalties + warn!("process_gossip_verified_execution_payload_envelope_failed") + } } } diff --git a/beacon_node/network/src/network_beacon_processor/mod.rs b/beacon_node/network/src/network_beacon_processor/mod.rs index fd67fcde82..ccd36b528e 100644 --- a/beacon_node/network/src/network_beacon_processor/mod.rs +++ b/beacon_node/network/src/network_beacon_processor/mod.rs @@ -429,11 +429,17 @@ impl NetworkBeaconProcessor { message_id: MessageId, peer_id: PeerId, execution_payload: Box>, + seen_timestamp: Duration, ) -> Result<(), Error> { let processor = self.clone(); let process_fn = async move { processor - .process_gossip_execution_payload(message_id, peer_id, *execution_payload) + .process_gossip_execution_payload_envelope( + message_id, + peer_id, + Arc::new(*execution_payload), + seen_timestamp, + ) .await }; diff --git a/beacon_node/network/src/router.rs b/beacon_node/network/src/router.rs index 8373dec322..77d64c92e6 100644 --- a/beacon_node/network/src/router.rs +++ b/beacon_node/network/src/router.rs @@ -493,6 +493,7 @@ impl Router { message_id, peer_id, signed_execution_payload_envelope, + timestamp_now(), ), ) } From 72fe22067ce49b5e3c92a8ce5f74a301f55083c1 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Fri, 13 Feb 2026 23:49:59 -0800 Subject: [PATCH 07/19] Import logs --- .../src/payload_envelope_verification/import.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs index 3628712721..7188793492 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -5,7 +5,7 @@ use fork_choice::PayloadVerificationStatus; use logging::crit; use slot_clock::SlotClock; use store::StoreOp; -use tracing::{debug, error, info_span, instrument}; +use tracing::{debug, error, info, info_span, instrument, warn}; use types::{BeaconState, BlockImportSource, Hash256, SignedBeaconBlock, Slot}; use super::{ @@ -111,11 +111,11 @@ impl BeaconChain { match import_envelope.await { // The block was successfully verified and imported. Yay. Ok(status @ AvailabilityProcessingStatus::Imported(block_root)) => { - debug!( + info!( ?block_root, %block_slot, source = %block_source, - "Envelope imported" + "Execution payload envelope imported" ); metrics::inc_counter(&metrics::ENVELOPE_PROCESSING_SUCCESSES); @@ -148,7 +148,7 @@ impl BeaconChain { } // The block failed verification. Err(other) => { - debug!(reason = other.to_string(), " Envelope rejected"); + warn!(reason = other.to_string(), "Execution payload envelope rejected"); Err(other) } } From de2362a8202572c59a5c81ba372de77aa34263d8 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Sun, 22 Feb 2026 10:17:47 -0800 Subject: [PATCH 08/19] Fix compilation error --- .../gossip_verified_envelope.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs index c9bef630aa..504a1d2c70 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs @@ -2,7 +2,10 @@ use std::sync::Arc; use educe::Educe; use slot_clock::SlotClock; -use state_processing::{VerifySignatures, envelope_processing::process_execution_payload_envelope}; +use state_processing::{ + VerifySignatures, + envelope_processing::{VerifyStateRoot, process_execution_payload_envelope}, +}; use tracing::{Span, debug}; use types::{ EthSpec, SignedBeaconBlock, SignedExecutionPayloadEnvelope, @@ -236,6 +239,7 @@ impl IntoExecutionPendingEnvelope for GossipVerifiedEnve &signed_envelope, // verify signature already done for GossipVerifiedEnvelope VerifySignatures::False, + VerifyStateRoot::True, &chain.spec, )?; From b525fe055fc854f591c6c5a01f33cf9aef48aa10 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Sun, 22 Feb 2026 16:07:48 -0800 Subject: [PATCH 09/19] Fix --- beacon_node/beacon_chain/src/beacon_chain.rs | 30 ------------------- .../beacon_chain/src/execution_payload.rs | 2 +- .../payload_envelope_verification/import.rs | 28 +++++++++++++++++ 3 files changed, 29 insertions(+), 31 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 1ed5579fea..a491e8559b 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -57,9 +57,6 @@ use crate::observed_block_producers::ObservedBlockProducers; use crate::observed_data_sidecars::ObservedDataSidecars; use crate::observed_operations::{ObservationOutcome, ObservedOperations}; use crate::observed_slashable::ObservedSlashable; -use crate::payload_envelope_verification::{ - EnvelopeError, ExecutedEnvelope, ExecutionPendingEnvelope, -}; use crate::pending_payload_envelopes::PendingPayloadEnvelopes; use crate::persisted_beacon_chain::PersistedBeaconChain; use crate::persisted_custody::persist_custody_context; @@ -3557,33 +3554,6 @@ impl BeaconChain { )) } - /// Accepts a fully-verified payload envelope and awaits on its payload verification handle to - /// get a fully `ExecutedEnvelope`. - /// - /// An error is returned if the verification handle couldn't be awaited. - #[instrument(skip_all, level = "debug")] - pub async fn into_executed_payload_envelope( - self: Arc, - pending_envelope: ExecutionPendingEnvelope, - ) -> Result, EnvelopeError> { - let ExecutionPendingEnvelope { - signed_envelope, - import_data, - payload_verification_handle, - } = pending_envelope; - - let payload_verification_outcome = payload_verification_handle - .await - .map_err(BeaconChainError::TokioJoin)? - .ok_or(BeaconChainError::RuntimeShutdown)??; - - Ok(ExecutedEnvelope::new( - signed_envelope, - import_data, - payload_verification_outcome, - )) - } - /* Import methods */ /// Checks if the block is available, and imports immediately if so, otherwise caches the block diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index b0644ac8aa..a5c2ead427 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -122,7 +122,7 @@ impl PayloadNotifier { } } -/// Verify that `execution_payload` contained by `block` is considered valid by an execution +/// Verify that `execution_payload` associated with `beacon_block_root` is considered valid by an execution /// engine. /// /// ## Specification diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs index f2633e8d5f..603e14446a 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -17,6 +17,7 @@ use crate::{ NotifyExecutionLayer, block_verification_types::{AsBlock, AvailableBlockData}, metrics, + payload_envelope_verification::ExecutionPendingEnvelope, validator_monitor::{get_slot_delay_ms, timestamp_now}, }; @@ -157,6 +158,33 @@ impl BeaconChain { } } + /// Accepts a fully-verified payload envelope and awaits on its payload verification handle to + /// get a fully `ExecutedEnvelope`. + /// + /// An error is returned if the verification handle couldn't be awaited. + #[instrument(skip_all, level = "debug")] + pub async fn into_executed_payload_envelope( + self: Arc, + pending_envelope: ExecutionPendingEnvelope, + ) -> Result, EnvelopeError> { + let ExecutionPendingEnvelope { + signed_envelope, + import_data, + payload_verification_handle, + } = pending_envelope; + + let payload_verification_outcome = payload_verification_handle + .await + .map_err(BeaconChainError::TokioJoin)? + .ok_or(BeaconChainError::RuntimeShutdown)??; + + Ok(ExecutedEnvelope::new( + signed_envelope, + import_data, + payload_verification_outcome, + )) + } + #[instrument(skip_all)] pub async fn import_available_execution_payload_envelope( self: &Arc, From d12bb4d712602fe2df0d3101863f4e33895fdb88 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Mon, 23 Feb 2026 12:06:15 -0800 Subject: [PATCH 10/19] Trying something out --- beacon_node/beacon_chain/src/beacon_chain.rs | 68 +-- .../beacon_chain/src/beacon_proposer_cache.rs | 79 ++- .../gossip_verified_envelope.rs | 79 ++- .../src/payload_envelope_verification/mod.rs | 19 +- .../payload_envelope_verification/tests.rs | 524 ++++++++++++++++++ 5 files changed, 680 insertions(+), 89 deletions(-) create mode 100644 beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index a491e8559b..4894bdaee9 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -4,9 +4,7 @@ use crate::attestation_verification::{ batch_verify_unaggregated_attestations, }; use crate::beacon_block_streamer::{BeaconBlockStreamer, CheckCaches}; -use crate::beacon_proposer_cache::{ - BeaconProposerCache, EpochBlockProposers, ensure_state_can_determine_proposers_for_epoch, -}; +use crate::beacon_proposer_cache::{BeaconProposerCache, EpochBlockProposers}; use crate::blob_verification::{GossipBlobError, GossipVerifiedBlob}; use crate::block_times_cache::BlockTimesCache; use crate::block_verification::POS_PANDA_BANNER; @@ -6548,62 +6546,14 @@ impl BeaconChain { accessor: impl Fn(&EpochBlockProposers) -> Result, state_provider: impl FnOnce() -> Result<(Hash256, BeaconState), E>, ) -> Result { - let cache_entry = self - .beacon_proposer_cache - .lock() - .get_or_insert_key(proposal_epoch, shuffling_decision_block); - - // If the cache entry is not initialised, run the code to initialise it inside a OnceCell. - // This prevents duplication of work across multiple threads. - // - // If it is already initialised, then `get_or_try_init` will return immediately without - // executing the initialisation code at all. - let epoch_block_proposers = cache_entry.get_or_try_init(|| { - // Fetch the state on-demand if the required epoch was missing from the cache. - // If the caller wants to not compute the state they must return an error here and then - // catch it at the call site. - let (state_root, mut state) = state_provider()?; - - // Ensure the state can compute proposer duties for `epoch`. - ensure_state_can_determine_proposers_for_epoch( - &mut state, - state_root, - proposal_epoch, - &self.spec, - )?; - - // Sanity check the state. - let latest_block_root = state.get_latest_block_root(state_root); - let state_decision_block_root = state.proposer_shuffling_decision_root_at_epoch( - proposal_epoch, - latest_block_root, - &self.spec, - )?; - if state_decision_block_root != shuffling_decision_block { - return Err(Error::ProposerCacheIncorrectState { - state_decision_block_root, - requested_decision_block_root: shuffling_decision_block, - } - .into()); - } - - let proposers = state.get_beacon_proposer_indices(proposal_epoch, &self.spec)?; - - // Use fork_at_epoch rather than the state's fork, because post-Fulu we may not have - // advanced the state completely into the new epoch. - let fork = self.spec.fork_at_epoch(proposal_epoch); - - debug!( - ?shuffling_decision_block, - epoch = %proposal_epoch, - "Priming proposer shuffling cache" - ); - - Ok::<_, E>(EpochBlockProposers::new(proposal_epoch, fork, proposers)) - })?; - - // Run the accessor function on the computed epoch proposers. - accessor(epoch_block_proposers).map_err(Into::into) + crate::beacon_proposer_cache::with_proposer_cache( + &self.beacon_proposer_cache, + &self.spec, + shuffling_decision_block, + proposal_epoch, + accessor, + state_provider, + ) } /// Runs the `map_fn` with the committee cache for `shuffling_epoch` from the chain with head diff --git a/beacon_node/beacon_chain/src/beacon_proposer_cache.rs b/beacon_node/beacon_chain/src/beacon_proposer_cache.rs index 912f7f3bad..141a79b202 100644 --- a/beacon_node/beacon_chain/src/beacon_proposer_cache.rs +++ b/beacon_node/beacon_chain/src/beacon_proposer_cache.rs @@ -12,12 +12,13 @@ use crate::{BeaconChain, BeaconChainError, BeaconChainTypes}; use fork_choice::ExecutionStatus; use lru::LruCache; use once_cell::sync::OnceCell; +use parking_lot::Mutex; use safe_arith::SafeArith; use smallvec::SmallVec; use state_processing::state_advance::partial_state_advance; use std::num::NonZeroUsize; use std::sync::Arc; -use tracing::instrument; +use tracing::{debug, instrument}; use typenum::Unsigned; use types::new_non_zero_usize; use types::{BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec, Fork, Hash256, Slot}; @@ -164,6 +165,82 @@ impl BeaconProposerCache { } } +/// Access the proposer cache, computing and caching the proposers if necessary. +/// +/// This is a free function that operates on references to the cache and spec, decoupled from +/// `BeaconChain`. The `accessor` is called with the cached `EpochBlockProposers` for the given +/// `(proposal_epoch, shuffling_decision_block)` key. If the cache entry is missing, the +/// `state_provider` closure is called to produce a state which is then used to compute and +/// cache the proposers. +pub fn with_proposer_cache( + beacon_proposer_cache: &Mutex, + spec: &ChainSpec, + shuffling_decision_block: Hash256, + proposal_epoch: Epoch, + accessor: impl Fn(&EpochBlockProposers) -> Result, + state_provider: impl FnOnce() -> Result<(Hash256, BeaconState), Err>, +) -> Result +where + Spec: EthSpec, + Err: From + From, +{ + let cache_entry = beacon_proposer_cache + .lock() + .get_or_insert_key(proposal_epoch, shuffling_decision_block); + + // If the cache entry is not initialised, run the code to initialise it inside a OnceCell. + // This prevents duplication of work across multiple threads. + // + // If it is already initialised, then `get_or_try_init` will return immediately without + // executing the initialisation code at all. + let epoch_block_proposers = cache_entry.get_or_try_init(|| { + // Fetch the state on-demand if the required epoch was missing from the cache. + // If the caller wants to not compute the state they must return an error here and then + // catch it at the call site. + let (state_root, mut state) = state_provider()?; + + // Ensure the state can compute proposer duties for `epoch`. + ensure_state_can_determine_proposers_for_epoch( + &mut state, + state_root, + proposal_epoch, + spec, + )?; + + // Sanity check the state. + let latest_block_root = state.get_latest_block_root(state_root); + let state_decision_block_root = state.proposer_shuffling_decision_root_at_epoch( + proposal_epoch, + latest_block_root, + spec, + )?; + if state_decision_block_root != shuffling_decision_block { + return Err(BeaconChainError::ProposerCacheIncorrectState { + state_decision_block_root, + requested_decision_block_root: shuffling_decision_block, + } + .into()); + } + + let proposers = state.get_beacon_proposer_indices(proposal_epoch, spec)?; + + // Use fork_at_epoch rather than the state's fork, because post-Fulu we may not have + // advanced the state completely into the new epoch. + let fork = spec.fork_at_epoch(proposal_epoch); + + debug!( + ?shuffling_decision_block, + epoch = %proposal_epoch, + "Priming proposer shuffling cache" + ); + + Ok::<_, Err>(EpochBlockProposers::new(proposal_epoch, fork, proposers)) + })?; + + // Run the accessor function on the computed epoch proposers. + accessor(epoch_block_proposers).map_err(Into::into) +} + /// Compute the proposer duties using the head state without cache. /// /// Return: diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs index 504a1d2c70..492b265fd0 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs @@ -1,27 +1,43 @@ use std::sync::Arc; use educe::Educe; +use parking_lot::{Mutex, RwLock}; use slot_clock::SlotClock; use state_processing::{ VerifySignatures, envelope_processing::{VerifyStateRoot, process_execution_payload_envelope}, }; +use store::DatabaseBlock; use tracing::{Span, debug}; use types::{ - EthSpec, SignedBeaconBlock, SignedExecutionPayloadEnvelope, + ChainSpec, EthSpec, Hash256, SignedBeaconBlock, SignedExecutionPayloadEnvelope, consts::gloas::BUILDER_INDEX_SELF_BUILD, }; use crate::{ - BeaconChain, BeaconChainError, BeaconChainTypes, NotifyExecutionLayer, + BeaconChain, BeaconChainError, BeaconChainTypes, BeaconStore, NotifyExecutionLayer, PayloadVerificationOutcome, + beacon_proposer_cache::{self, BeaconProposerCache}, + canonical_head::CanonicalHead, payload_envelope_verification::{ EnvelopeError, EnvelopeImportData, EnvelopeProcessingSnapshot, ExecutionPendingEnvelope, IntoExecutionPendingEnvelope, MaybeAvailableEnvelope, load_snapshot, payload_notifier::PayloadNotifier, }, + validator_pubkey_cache::ValidatorPubkeyCache, }; +/// Bundles only the dependencies needed for gossip verification of execution payload envelopes, +/// decoupling `GossipVerifiedEnvelope::new` from the full `BeaconChain`. +pub struct GossipVerificationContext<'a, T: BeaconChainTypes> { + pub canonical_head: &'a CanonicalHead, + pub store: &'a BeaconStore, + pub spec: &'a ChainSpec, + pub beacon_proposer_cache: &'a Mutex, + pub validator_pubkey_cache: &'a RwLock>, + pub genesis_validators_root: Hash256, +} + /// A wrapper around a `SignedExecutionPayloadEnvelope` that indicates it has been approved for re-gossiping on /// the p2p network. #[derive(Educe)] @@ -35,7 +51,7 @@ pub struct GossipVerifiedEnvelope { impl GossipVerifiedEnvelope { pub fn new( signed_envelope: Arc>, - chain: &BeaconChain, + ctx: &GossipVerificationContext<'_, T>, ) -> Result { let envelope = &signed_envelope.message; let payload = &envelope.payload; @@ -48,7 +64,7 @@ impl GossipVerifiedEnvelope { // 2. Blocks we've seen that are invalid (REJECT). // // Presently these two cases are conflated. - let fork_choice_read_lock = chain.canonical_head.fork_choice_read_lock(); + let fork_choice_read_lock = ctx.canonical_head.fork_choice_read_lock(); let Some(proto_block) = fork_choice_read_lock.get_block(&beacon_block_root) else { return Err(EnvelopeError::BlockRootUnknown { block_root: beacon_block_root, @@ -64,12 +80,14 @@ impl GossipVerifiedEnvelope { // TODO(EIP-7732): check that we haven't seen another valid `SignedExecutionPayloadEnvelope` // for this block root from this builder - envelope status table check - let block = chain - .get_full_block(&beacon_block_root)? - .ok_or_else(|| { - EnvelopeError::from(BeaconChainError::MissingBeaconBlock(beacon_block_root)) - }) - .map(Arc::new)?; + let block = match ctx.store.try_get_full_block(&beacon_block_root)? { + Some(DatabaseBlock::Full(block)) => Arc::new(block), + Some(DatabaseBlock::Blinded(_)) | None => { + return Err(EnvelopeError::from(BeaconChainError::MissingBeaconBlock( + beacon_block_root, + ))); + } + }; let execution_bid = &block .message() .body() @@ -118,13 +136,15 @@ impl GossipVerifiedEnvelope { let block_slot = envelope.slot; let block_epoch = block_slot.epoch(T::EthSpec::slots_per_epoch()); let proposer_shuffling_decision_block = - proto_block.proposer_shuffling_root_for_child_block(block_epoch, &chain.spec); + proto_block.proposer_shuffling_root_for_child_block(block_epoch, ctx.spec); let (signature_is_valid, opt_snapshot) = if builder_index == BUILDER_INDEX_SELF_BUILD { // Fast path: self-build envelopes can be verified without loading the state. let envelope_ref = signed_envelope.as_ref(); let mut opt_snapshot = None; - let proposer = chain.with_proposer_cache::<_, EnvelopeError>( + let proposer = beacon_proposer_cache::with_proposer_cache( + ctx.beacon_proposer_cache, + ctx.spec, proposer_shuffling_decision_block, block_epoch, |proposers| proposers.get_slot::(block_slot), @@ -133,14 +153,14 @@ impl GossipVerifiedEnvelope { %beacon_block_root, "Proposer shuffling cache miss for envelope verification" ); - let snapshot = load_snapshot(envelope_ref, chain)?; + let snapshot = load_snapshot(envelope_ref, ctx.canonical_head, ctx.store)?; opt_snapshot = Some(Box::new(snapshot.clone())); - Ok((snapshot.state_root, snapshot.pre_state)) + Ok::<_, EnvelopeError>((snapshot.state_root, snapshot.pre_state)) }, )?; let fork = proposer.fork; - let pubkey_cache = chain.validator_pubkey_cache.read(); + let pubkey_cache = ctx.validator_pubkey_cache.read(); let pubkey = pubkey_cache .get(block.message().proposer_index() as usize) .ok_or_else(|| EnvelopeError::UnknownValidator { @@ -149,16 +169,16 @@ impl GossipVerifiedEnvelope { let is_valid = signed_envelope.verify_signature( pubkey, &fork, - chain.genesis_validators_root, - &chain.spec, + ctx.genesis_validators_root, + ctx.spec, ); (is_valid, opt_snapshot) } else { // TODO(gloas) if we implement a builder pubkey cache, we'll need to use it here. // External builder: must load the state to get the builder pubkey. - let snapshot = load_snapshot(signed_envelope.as_ref(), chain)?; + let snapshot = load_snapshot(signed_envelope.as_ref(), ctx.canonical_head, ctx.store)?; let is_valid = - signed_envelope.verify_signature_with_state(&snapshot.pre_state, &chain.spec)?; + signed_envelope.verify_signature_with_state(&snapshot.pre_state, ctx.spec)?; (is_valid, Some(Box::new(snapshot))) }; @@ -228,7 +248,11 @@ impl IntoExecutionPendingEnvelope for GossipVerifiedEnve let snapshot = if let Some(snapshot) = self.snapshot { *snapshot } else { - load_snapshot(signed_envelope.as_ref(), chain)? + load_snapshot( + signed_envelope.as_ref(), + &chain.canonical_head, + &chain.store, + )? }; let mut state = snapshot.pre_state; @@ -263,6 +287,18 @@ impl IntoExecutionPendingEnvelope for GossipVerifiedEnve } impl BeaconChain { + /// Build a `GossipVerificationContext` from this `BeaconChain`. + pub fn gossip_verification_context(&self) -> GossipVerificationContext<'_, T> { + GossipVerificationContext { + canonical_head: &self.canonical_head, + store: &self.store, + spec: &self.spec, + beacon_proposer_cache: &self.beacon_proposer_cache, + validator_pubkey_cache: &self.validator_pubkey_cache, + genesis_validators_root: self.genesis_validators_root, + } + } + /// Returns `Ok(GossipVerifiedEnvelope)` if the supplied `envelope` should be forwarded onto the /// gossip network. The envelope is not imported into the chain, it is just partially verified. /// @@ -287,7 +323,8 @@ impl BeaconChain { let slot = envelope.slot(); let beacon_block_root = envelope.message.beacon_block_root; - match GossipVerifiedEnvelope::new(envelope, &chain) { + let ctx = chain.gossip_verification_context(); + match GossipVerifiedEnvelope::new(envelope, &ctx) { Ok(verified) => { debug!( %slot, diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs index 80e62f93b7..38fdd9f425 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs @@ -39,9 +39,9 @@ use types::{ }; use crate::{ - BeaconChain, BeaconChainError, BeaconChainTypes, BlockError, ExecutionPayloadError, - NotifyExecutionLayer, PayloadVerificationOutcome, - block_verification::PayloadVerificationHandle, + BeaconChain, BeaconChainError, BeaconChainTypes, BeaconStore, BlockError, + ExecutionPayloadError, NotifyExecutionLayer, PayloadVerificationOutcome, + block_verification::PayloadVerificationHandle, canonical_head::CanonicalHead, payload_envelope_verification::gossip_verified_envelope::GossipVerifiedEnvelope, }; @@ -49,6 +49,9 @@ pub mod gossip_verified_envelope; pub mod import; mod payload_notifier; +#[cfg(test)] +mod tests; + pub trait IntoExecutionPendingEnvelope: Sized { fn into_execution_pending_envelope( self, @@ -289,7 +292,8 @@ impl From for EnvelopeError { #[instrument(skip_all, level = "debug", fields(beacon_block_root = %envelope.beacon_block_root()))] pub(crate) fn load_snapshot( envelope: &SignedExecutionPayloadEnvelope, - chain: &BeaconChain, + canonical_head: &CanonicalHead, + store: &BeaconStore, ) -> Result, EnvelopeError> { // Reject any envelope if its block is not known to fork choice. // @@ -302,7 +306,7 @@ pub(crate) fn load_snapshot( // choice, so we will not reject any child of the finalized block (this is relevant during // genesis). - let fork_choice_read_lock = chain.canonical_head.fork_choice_read_lock(); + let fork_choice_read_lock = canonical_head.fork_choice_read_lock(); let beacon_block_root = envelope.beacon_block_root(); let Some(proto_beacon_block) = fork_choice_read_lock.get_block(&beacon_block_root) else { return Err(EnvelopeError::BlockRootUnknown { @@ -317,8 +321,7 @@ pub(crate) fn load_snapshot( // We can use `get_hot_state` here rather than `get_advanced_hot_state` because the envelope // must be from the same slot as its block (so no advance is required). let cache_state = true; - let state = chain - .store + let state = store .get_hot_state(&block_state_root, cache_state) .map_err(EnvelopeError::from)? .ok_or_else(|| { @@ -342,7 +345,7 @@ impl IntoExecutionPendingEnvelope chain: &Arc>, notify_execution_layer: NotifyExecutionLayer, ) -> Result, EnvelopeError> { - GossipVerifiedEnvelope::new(self, chain)? + GossipVerifiedEnvelope::new(self, &chain.gossip_verification_context())? .into_execution_pending_envelope(chain, notify_execution_layer) } diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs new file mode 100644 index 0000000000..c362bc6180 --- /dev/null +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs @@ -0,0 +1,524 @@ +use std::sync::Arc; +use std::time::Duration; + +use bls::{FixedBytesExtended, Keypair, Signature}; +use fork_choice::ForkChoice; +use parking_lot::{Mutex, RwLock}; +use ssz_types::VariableList; +use store::{HotColdDB, KeyValueStore, MemoryStore, StoreConfig}; +use types::consts::gloas::BUILDER_INDEX_SELF_BUILD; +use types::test_utils::generate_deterministic_keypairs; +use types::*; + +use crate::BeaconStore; +use crate::beacon_fork_choice_store::BeaconForkChoiceStore; +use crate::beacon_proposer_cache::BeaconProposerCache; +use crate::builder::Witness; +use crate::canonical_head::CanonicalHead; +use crate::payload_envelope_verification::EnvelopeError; +use crate::payload_envelope_verification::gossip_verified_envelope::{ + GossipVerificationContext, GossipVerifiedEnvelope, +}; +use crate::validator_pubkey_cache::ValidatorPubkeyCache; + +type TestEthSpec = MinimalEthSpec; +type TestTypes = Witness< + slot_clock::TestingSlotClock, + TestEthSpec, + MemoryStore, + MemoryStore, +>; + +/// Test context that holds the minimal state needed for gossip verification. +struct TestContext { + store: BeaconStore, + canonical_head: CanonicalHead, + beacon_proposer_cache: Mutex, + validator_pubkey_cache: RwLock>, + spec: Arc, + keypairs: Vec, + genesis_state: BeaconState, + genesis_block_root: Hash256, + genesis_validators_root: Hash256, +} + +impl TestContext { + fn new(validator_count: usize) -> Self { + let spec = Arc::new(ForkName::Gloas.make_genesis_spec(ChainSpec::minimal())); + let keypairs = generate_deterministic_keypairs(validator_count); + + let mut genesis_state = genesis::interop_genesis_state::( + &keypairs, + 0, // genesis_time + Hash256::from_slice(&[0x42; 32]), + None, // no execution payload header + &spec, + ) + .expect("should create genesis state"); + + let genesis_validators_root = genesis_state.genesis_validators_root(); + + let store: BeaconStore = Arc::new( + HotColdDB::open_ephemeral(StoreConfig::default(), spec.clone()) + .expect("should create ephemeral store"), + ); + + // Initialize store metadata. + let genesis_block = BeaconBlock::::empty(&spec); + let genesis_block_root = genesis_block.canonical_root(); + let signed_genesis_block = SignedBeaconBlock::from_block(genesis_block, Signature::empty()); + + // Build caches and compute state root before storing. + genesis_state + .build_caches(&spec) + .expect("should build caches"); + + // Initialize store metadata ops (must be done before put_state). + let ops = vec![ + store + .init_anchor_info( + signed_genesis_block.parent_root(), + signed_genesis_block.slot(), + Slot::new(0), + false, + ) + .expect("should init anchor info"), + store + .init_blob_info(signed_genesis_block.slot()) + .expect("should init blob info"), + store + .init_data_column_info(signed_genesis_block.slot()) + .expect("should init data column info"), + ]; + store + .hot_db + .do_atomically(ops) + .expect("should store metadata"); + + // Store the genesis block and state. + store + .put_block(&genesis_block_root, signed_genesis_block.clone()) + .expect("should store genesis block"); + let state_root = genesis_state + .update_tree_hash_cache() + .expect("should compute state root"); + store + .put_state(&state_root, &genesis_state) + .expect("should store genesis state"); + + // Create BeaconSnapshot and fork choice. + let snapshot = crate::BeaconSnapshot { + beacon_block: Arc::new(signed_genesis_block), + beacon_block_root: genesis_block_root, + beacon_state: genesis_state.clone(), + }; + + let fc_store = BeaconForkChoiceStore::get_forkchoice_store(store.clone(), snapshot.clone()) + .expect("should create fork choice store"); + + let fork_choice = ForkChoice::from_anchor( + fc_store, + genesis_block_root, + &snapshot.beacon_block, + &snapshot.beacon_state, + None, + &spec, + ) + .expect("should create fork choice from anchor"); + + let canonical_head = CanonicalHead::new(fork_choice, Arc::new(snapshot)); + + let validator_pubkey_cache = ValidatorPubkeyCache::new(&genesis_state, store.clone()) + .expect("should create validator pubkey cache"); + + TestContext { + store, + canonical_head, + beacon_proposer_cache: Mutex::new(BeaconProposerCache::default()), + validator_pubkey_cache: RwLock::new(validator_pubkey_cache), + spec, + keypairs, + genesis_state, + genesis_block_root, + genesis_validators_root, + } + } + + fn gossip_verification_context(&self) -> GossipVerificationContext<'_, TestTypes> { + GossipVerificationContext { + canonical_head: &self.canonical_head, + store: &self.store, + spec: &self.spec, + beacon_proposer_cache: &self.beacon_proposer_cache, + validator_pubkey_cache: &self.validator_pubkey_cache, + genesis_validators_root: self.genesis_validators_root, + } + } + + /// Build a gloas block at `slot` with the given proposer, store it, add it to fork choice, + /// and return the signed block, block root, and post-state. + fn build_and_import_block( + &self, + slot: Slot, + proposer_index: usize, + execution_bid: ExecutionPayloadBid, + ) -> ( + Arc>, + Hash256, + BeaconState, + ) { + let mut state = self.genesis_state.clone(); + + // Advance the state to the target slot. + if slot > state.slot() { + state_processing::state_advance::complete_state_advance( + &mut state, None, slot, &self.spec, + ) + .expect("should advance state"); + } + + state.build_caches(&self.spec).expect("should build caches"); + + // Compute the state root so we can embed it in the block. + let state_root = state + .update_tree_hash_cache() + .expect("should compute state root"); + + let signed_bid = SignedExecutionPayloadBid { + message: execution_bid, + signature: Signature::infinity().expect("should create infinity signature"), + }; + + // Create the block body with the actual state root. + let block = BeaconBlock::Gloas(BeaconBlockGloas { + slot, + proposer_index: proposer_index as u64, + parent_root: self.genesis_block_root, + state_root, + body: BeaconBlockBodyGloas { + randao_reveal: Signature::empty(), + eth1_data: state.eth1_data().clone(), + graffiti: Graffiti::default(), + proposer_slashings: VariableList::empty(), + attester_slashings: VariableList::empty(), + attestations: VariableList::empty(), + deposits: VariableList::empty(), + voluntary_exits: VariableList::empty(), + sync_aggregate: SyncAggregate::empty(), + bls_to_execution_changes: VariableList::empty(), + signed_execution_payload_bid: signed_bid, + payload_attestations: VariableList::empty(), + _phantom: std::marker::PhantomData, + }, + }); + + let block_root = block.canonical_root(); + let proposer_sk = &self.keypairs[proposer_index].sk; + let fork = self + .spec + .fork_at_epoch(slot.epoch(TestEthSpec::slots_per_epoch())); + let signed_block = block.sign(proposer_sk, &fork, self.genesis_validators_root, &self.spec); + + // Store block and state. + self.store + .put_block(&block_root, signed_block.clone()) + .expect("should store block"); + self.store + .put_state(&state_root, &state) + .expect("should store state"); + + // Add block to fork choice. + let mut fork_choice = self.canonical_head.fork_choice_write_lock(); + fork_choice + .on_block( + slot, + signed_block.message(), + block_root, + Duration::from_secs(0), + &state, + crate::PayloadVerificationStatus::Verified, + &self.spec, + ) + .expect("should add block to fork choice"); + drop(fork_choice); + + (Arc::new(signed_block), block_root, state) + } + + /// Build a signed execution payload envelope for the given block. + fn build_signed_envelope( + &self, + block_root: Hash256, + slot: Slot, + builder_index: u64, + block_hash: ExecutionBlockHash, + signing_key: &bls::SecretKey, + ) -> Arc> { + let mut payload = ExecutionPayloadGloas::default(); + payload.block_hash = block_hash; + + let envelope = ExecutionPayloadEnvelope { + payload, + execution_requests: ExecutionRequests::default(), + builder_index, + beacon_block_root: block_root, + slot, + state_root: Hash256::zero(), + }; + + let fork = self + .spec + .fork_at_epoch(slot.epoch(TestEthSpec::slots_per_epoch())); + let domain = self.spec.get_domain( + slot.epoch(TestEthSpec::slots_per_epoch()), + Domain::BeaconBuilder, + &fork, + self.genesis_validators_root, + ); + let message = envelope.signing_root(domain); + let signature = signing_key.sign(message); + + Arc::new(SignedExecutionPayloadEnvelope { + message: envelope, + signature, + }) + } + + /// Helper: build a block and matching self-build envelope. + fn build_block_and_envelope( + &self, + ) -> ( + Arc>, + Hash256, + Arc>, + ) { + let slot = Slot::new(1); + let block_hash = ExecutionBlockHash::from_root(Hash256::from_slice(&[0xaa; 32])); + + // Get proposer for slot 1. + let mut state = self.genesis_state.clone(); + state_processing::state_advance::complete_state_advance(&mut state, None, slot, &self.spec) + .expect("should advance state"); + state.build_caches(&self.spec).expect("should build caches"); + let proposer_index = state + .get_beacon_proposer_index(slot, &self.spec) + .expect("should get proposer index"); + + let bid = ExecutionPayloadBid { + builder_index: BUILDER_INDEX_SELF_BUILD, + block_hash, + slot, + ..Default::default() + }; + + let (signed_block, block_root, _post_state) = + self.build_and_import_block(slot, proposer_index, bid); + + let proposer_sk = &self.keypairs[proposer_index].sk; + let envelope = self.build_signed_envelope( + block_root, + slot, + BUILDER_INDEX_SELF_BUILD, + block_hash, + proposer_sk, + ); + + (signed_block, block_root, envelope) + } +} + +#[test] +fn test_valid_self_build_envelope() { + let ctx = TestContext::new(32); + let (_block, _block_root, envelope) = ctx.build_block_and_envelope(); + let gossip_ctx = ctx.gossip_verification_context(); + + let result = GossipVerifiedEnvelope::new(envelope, &gossip_ctx); + assert!( + result.is_ok(), + "valid self-build envelope should pass verification, got: {:?}", + result.err() + ); +} + +#[test] +fn test_unknown_block_root() { + let ctx = TestContext::new(32); + let gossip_ctx = ctx.gossip_verification_context(); + + // Build an envelope referencing a block root not in fork choice. + let unknown_root = Hash256::from_slice(&[0xff; 32]); + let envelope = ctx.build_signed_envelope( + unknown_root, + Slot::new(1), + BUILDER_INDEX_SELF_BUILD, + ExecutionBlockHash::from_root(Hash256::zero()), + &ctx.keypairs[0].sk, + ); + + let result = GossipVerifiedEnvelope::new(envelope, &gossip_ctx); + assert!( + matches!(result, Err(EnvelopeError::BlockRootUnknown { .. })), + "should reject envelope with unknown block root, got: {:?}", + result + ); +} + +#[test] +fn test_slot_mismatch() { + let ctx = TestContext::new(32); + let (_block, block_root, _good_envelope) = ctx.build_block_and_envelope(); + let gossip_ctx = ctx.gossip_verification_context(); + + // Build an envelope with a different slot than the block. + let wrong_slot = Slot::new(2); + let envelope = ctx.build_signed_envelope( + block_root, + wrong_slot, + BUILDER_INDEX_SELF_BUILD, + ExecutionBlockHash::from_root(Hash256::from_slice(&[0xaa; 32])), + &ctx.keypairs[0].sk, + ); + + let result = GossipVerifiedEnvelope::new(envelope, &gossip_ctx); + assert!( + matches!(result, Err(EnvelopeError::SlotMismatch { .. })), + "should reject envelope with slot mismatch, got: {:?}", + result + ); +} + +#[test] +fn test_builder_index_mismatch() { + let ctx = TestContext::new(32); + let gossip_ctx = ctx.gossip_verification_context(); + + let slot = Slot::new(1); + let block_hash = ExecutionBlockHash::from_root(Hash256::from_slice(&[0xaa; 32])); + + // Get proposer for slot 1. + let mut state = ctx.genesis_state.clone(); + state_processing::state_advance::complete_state_advance(&mut state, None, slot, &ctx.spec) + .expect("should advance state"); + state.build_caches(&ctx.spec).expect("should build caches"); + let proposer_index = state + .get_beacon_proposer_index(slot, &ctx.spec) + .expect("should get proposer index"); + + // Block has builder_index = BUILDER_INDEX_SELF_BUILD + let bid = ExecutionPayloadBid { + builder_index: BUILDER_INDEX_SELF_BUILD, + block_hash, + slot, + ..Default::default() + }; + let (_block, block_root, _post_state) = ctx.build_and_import_block(slot, proposer_index, bid); + + // Envelope has a different builder_index. + let wrong_builder_index = 999; + let envelope = ctx.build_signed_envelope( + block_root, + slot, + wrong_builder_index, + block_hash, + &ctx.keypairs[proposer_index].sk, + ); + + let result = GossipVerifiedEnvelope::new(envelope, &gossip_ctx); + assert!( + matches!(result, Err(EnvelopeError::BuilderIndexMismatch { .. })), + "should reject envelope with builder index mismatch, got: {:?}", + result + ); +} + +#[test] +fn test_block_hash_mismatch() { + let ctx = TestContext::new(32); + let gossip_ctx = ctx.gossip_verification_context(); + + let slot = Slot::new(1); + let block_hash = ExecutionBlockHash::from_root(Hash256::from_slice(&[0xaa; 32])); + let wrong_block_hash = ExecutionBlockHash::from_root(Hash256::from_slice(&[0xbb; 32])); + + // Get proposer for slot 1. + let mut state = ctx.genesis_state.clone(); + state_processing::state_advance::complete_state_advance(&mut state, None, slot, &ctx.spec) + .expect("should advance state"); + state.build_caches(&ctx.spec).expect("should build caches"); + let proposer_index = state + .get_beacon_proposer_index(slot, &ctx.spec) + .expect("should get proposer index"); + + // Block has block_hash = 0xaa + let bid = ExecutionPayloadBid { + builder_index: BUILDER_INDEX_SELF_BUILD, + block_hash, + slot, + ..Default::default() + }; + let (_block, block_root, _post_state) = ctx.build_and_import_block(slot, proposer_index, bid); + + // Envelope has a different block_hash. + let envelope = ctx.build_signed_envelope( + block_root, + slot, + BUILDER_INDEX_SELF_BUILD, + wrong_block_hash, + &ctx.keypairs[proposer_index].sk, + ); + + let result = GossipVerifiedEnvelope::new(envelope, &gossip_ctx); + assert!( + matches!(result, Err(EnvelopeError::BlockHashMismatch { .. })), + "should reject envelope with block hash mismatch, got: {:?}", + result + ); +} + +#[test] +fn test_bad_signature() { + let ctx = TestContext::new(32); + let gossip_ctx = ctx.gossip_verification_context(); + + let slot = Slot::new(1); + let block_hash = ExecutionBlockHash::from_root(Hash256::from_slice(&[0xaa; 32])); + + // Get proposer for slot 1. + let mut state = ctx.genesis_state.clone(); + state_processing::state_advance::complete_state_advance(&mut state, None, slot, &ctx.spec) + .expect("should advance state"); + state.build_caches(&ctx.spec).expect("should build caches"); + let proposer_index = state + .get_beacon_proposer_index(slot, &ctx.spec) + .expect("should get proposer index"); + + let bid = ExecutionPayloadBid { + builder_index: BUILDER_INDEX_SELF_BUILD, + block_hash, + slot, + ..Default::default() + }; + let (_block, block_root, _post_state) = ctx.build_and_import_block(slot, proposer_index, bid); + + // Sign the envelope with the wrong key (some other validator's key). + let wrong_key_index = if proposer_index == 0 { 1 } else { 0 }; + let envelope = ctx.build_signed_envelope( + block_root, + slot, + BUILDER_INDEX_SELF_BUILD, + block_hash, + &ctx.keypairs[wrong_key_index].sk, + ); + + let result = GossipVerifiedEnvelope::new(envelope, &gossip_ctx); + assert!( + matches!(result, Err(EnvelopeError::BadSignature)), + "should reject envelope with bad signature, got: {:?}", + result + ); +} + +// NOTE: `test_prior_to_finalization` is omitted here because advancing finalization requires +// attestation-based justification which needs the full `BeaconChainHarness`. The +// `PriorToFinalization` code path is tested in the integration tests. From 8ce81578b7ee61aad396fd2a62a82497139a2570 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Tue, 24 Feb 2026 10:46:46 -0800 Subject: [PATCH 11/19] introduce a smalll refactor and unit test --- .../gossip_verified_envelope.rs | 237 ++++++-- .../src/payload_envelope_verification/mod.rs | 3 - .../payload_envelope_verification/tests.rs | 524 ------------------ 3 files changed, 202 insertions(+), 562 deletions(-) delete mode 100644 beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs index 492b265fd0..9d555e8ad2 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs @@ -10,8 +10,8 @@ use state_processing::{ use store::DatabaseBlock; use tracing::{Span, debug}; use types::{ - ChainSpec, EthSpec, Hash256, SignedBeaconBlock, SignedExecutionPayloadEnvelope, - consts::gloas::BUILDER_INDEX_SELF_BUILD, + ChainSpec, EthSpec, ExecutionPayloadBid, ExecutionPayloadEnvelope, Hash256, SignedBeaconBlock, + SignedExecutionPayloadEnvelope, Slot, consts::gloas::BUILDER_INDEX_SELF_BUILD, }; use crate::{ @@ -38,6 +38,54 @@ pub struct GossipVerificationContext<'a, T: BeaconChainTypes> { pub genesis_validators_root: Hash256, } +/// Verify that an execution payload envelope is consistent with its beacon block +/// and execution bid. This checks: +/// - The envelope slot is not prior to finalization +/// - The envelope slot matches the block slot +/// - The builder index matches the committed bid +/// - The payload block hash matches the committed bid +pub(crate) fn verify_envelope_consistency( + envelope: &ExecutionPayloadEnvelope, + block: &SignedBeaconBlock, + execution_bid: &ExecutionPayloadBid, + latest_finalized_slot: Slot, +) -> Result<(), EnvelopeError> { + // Check that the envelope's slot isn't from a slot prior + // to the latest finalized slot. + if envelope.slot < latest_finalized_slot { + return Err(EnvelopeError::PriorToFinalization { + payload_slot: envelope.slot, + latest_finalized_slot, + }); + } + + // Check that the slot of the envelope matches the slot of the parent block. + if envelope.slot != block.slot() { + return Err(EnvelopeError::SlotMismatch { + block: block.slot(), + envelope: envelope.slot, + }); + } + + // Builder index matches committed bid. + if envelope.builder_index != execution_bid.builder_index { + return Err(EnvelopeError::BuilderIndexMismatch { + committed_bid: execution_bid.builder_index, + envelope: envelope.builder_index, + }); + } + + // The block hash should match the block hash of the execution bid. + if envelope.payload.block_hash != execution_bid.block_hash { + return Err(EnvelopeError::BlockHashMismatch { + committed_bid: execution_bid.block_hash, + envelope: envelope.payload.block_hash, + }); + } + + Ok(()) +} + /// A wrapper around a `SignedExecutionPayloadEnvelope` that indicates it has been approved for re-gossiping on /// the p2p network. #[derive(Educe)] @@ -54,7 +102,6 @@ impl GossipVerifiedEnvelope { ctx: &GossipVerificationContext<'_, T>, ) -> Result { let envelope = &signed_envelope.message; - let payload = &envelope.payload; let beacon_block_root = envelope.beacon_block_root; // Check that we've seen the beacon block for this envelope and that it passes validation. @@ -94,38 +141,7 @@ impl GossipVerifiedEnvelope { .signed_execution_payload_bid()? .message; - // check that the envelopes slot isnt from a slot prior - // to the latest finalized slot. - if envelope.slot < latest_finalized_slot { - return Err(EnvelopeError::PriorToFinalization { - payload_slot: envelope.slot, - latest_finalized_slot, - }); - } - - // check that the slot of the envelope matches the slot of the parent block - if envelope.slot != block.slot() { - return Err(EnvelopeError::SlotMismatch { - block: block.slot(), - envelope: envelope.slot, - }); - } - - // builder index matches committed bid - if envelope.builder_index != execution_bid.builder_index { - return Err(EnvelopeError::BuilderIndexMismatch { - committed_bid: execution_bid.builder_index, - envelope: envelope.builder_index, - }); - } - - // the block hash should match the block hash of the execution bid - if payload.block_hash != execution_bid.block_hash { - return Err(EnvelopeError::BlockHashMismatch { - committed_bid: execution_bid.block_hash, - envelope: payload.block_hash, - }); - } + verify_envelope_consistency(envelope, &block, execution_bid, latest_finalized_slot)?; // Verify the envelope signature. // @@ -353,3 +369,154 @@ impl BeaconChain { .map_err(BeaconChainError::TokioJoin)? } } + +#[cfg(test)] +mod tests { + use std::marker::PhantomData; + + use bls::Signature; + use ssz_types::VariableList; + use types::{ + BeaconBlock, BeaconBlockBodyGloas, BeaconBlockGloas, Eth1Data, ExecutionBlockHash, + ExecutionPayloadBid, ExecutionPayloadEnvelope, ExecutionPayloadGloas, ExecutionRequests, + Graffiti, Hash256, MinimalEthSpec, SignedBeaconBlock, SignedExecutionPayloadBid, Slot, + SyncAggregate, + }; + + use super::verify_envelope_consistency; + use crate::payload_envelope_verification::EnvelopeError; + + type E = MinimalEthSpec; + + fn make_envelope( + slot: Slot, + builder_index: u64, + block_hash: ExecutionBlockHash, + ) -> ExecutionPayloadEnvelope { + ExecutionPayloadEnvelope { + payload: ExecutionPayloadGloas { + block_hash, + ..ExecutionPayloadGloas::default() + }, + execution_requests: ExecutionRequests::default(), + builder_index, + beacon_block_root: Hash256::ZERO, + slot, + state_root: Hash256::ZERO, + } + } + + fn make_block(slot: Slot) -> SignedBeaconBlock { + let block = BeaconBlock::Gloas(BeaconBlockGloas { + slot, + proposer_index: 0, + parent_root: Hash256::ZERO, + state_root: Hash256::ZERO, + body: BeaconBlockBodyGloas { + randao_reveal: Signature::empty(), + eth1_data: Eth1Data { + deposit_root: Hash256::ZERO, + block_hash: Hash256::ZERO, + deposit_count: 0, + }, + graffiti: Graffiti::default(), + proposer_slashings: VariableList::empty(), + attester_slashings: VariableList::empty(), + attestations: VariableList::empty(), + deposits: VariableList::empty(), + voluntary_exits: VariableList::empty(), + sync_aggregate: SyncAggregate::empty(), + bls_to_execution_changes: VariableList::empty(), + signed_execution_payload_bid: SignedExecutionPayloadBid::empty(), + payload_attestations: VariableList::empty(), + _phantom: PhantomData, + }, + }); + SignedBeaconBlock::from_block(block, Signature::empty()) + } + + fn make_bid(builder_index: u64, block_hash: ExecutionBlockHash) -> ExecutionPayloadBid { + ExecutionPayloadBid { + builder_index, + block_hash, + ..ExecutionPayloadBid::default() + } + } + + #[test] + fn test_valid_envelope() { + let slot = Slot::new(10); + let builder_index = 5; + let block_hash = ExecutionBlockHash::repeat_byte(0xaa); + + let envelope = make_envelope(slot, builder_index, block_hash); + let block = make_block(slot); + let bid = make_bid(builder_index, block_hash); + + assert!(verify_envelope_consistency::(&envelope, &block, &bid, Slot::new(0)).is_ok()); + } + + #[test] + fn test_prior_to_finalization() { + let slot = Slot::new(5); + let builder_index = 1; + let block_hash = ExecutionBlockHash::repeat_byte(0xbb); + + let envelope = make_envelope(slot, builder_index, block_hash); + let block = make_block(slot); + let bid = make_bid(builder_index, block_hash); + let latest_finalized_slot = Slot::new(10); + + let result = + verify_envelope_consistency::(&envelope, &block, &bid, latest_finalized_slot); + assert!(matches!( + result, + Err(EnvelopeError::PriorToFinalization { .. }) + )); + } + + #[test] + fn test_slot_mismatch() { + let builder_index = 1; + let block_hash = ExecutionBlockHash::repeat_byte(0xcc); + + let envelope = make_envelope(Slot::new(10), builder_index, block_hash); + let block = make_block(Slot::new(20)); + let bid = make_bid(builder_index, block_hash); + + let result = verify_envelope_consistency::(&envelope, &block, &bid, Slot::new(0)); + assert!(matches!(result, Err(EnvelopeError::SlotMismatch { .. }))); + } + + #[test] + fn test_builder_index_mismatch() { + let slot = Slot::new(10); + let block_hash = ExecutionBlockHash::repeat_byte(0xdd); + + let envelope = make_envelope(slot, 1, block_hash); + let block = make_block(slot); + let bid = make_bid(2, block_hash); + + let result = verify_envelope_consistency::(&envelope, &block, &bid, Slot::new(0)); + assert!(matches!( + result, + Err(EnvelopeError::BuilderIndexMismatch { .. }) + )); + } + + #[test] + fn test_block_hash_mismatch() { + let slot = Slot::new(10); + let builder_index = 1; + + let envelope = make_envelope(slot, builder_index, ExecutionBlockHash::repeat_byte(0xee)); + let block = make_block(slot); + let bid = make_bid(builder_index, ExecutionBlockHash::repeat_byte(0xff)); + + let result = verify_envelope_consistency::(&envelope, &block, &bid, Slot::new(0)); + assert!(matches!( + result, + Err(EnvelopeError::BlockHashMismatch { .. }) + )); + } +} diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs index 38fdd9f425..ae5dbfccc0 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs @@ -49,9 +49,6 @@ pub mod gossip_verified_envelope; pub mod import; mod payload_notifier; -#[cfg(test)] -mod tests; - pub trait IntoExecutionPendingEnvelope: Sized { fn into_execution_pending_envelope( self, diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs deleted file mode 100644 index c362bc6180..0000000000 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/tests.rs +++ /dev/null @@ -1,524 +0,0 @@ -use std::sync::Arc; -use std::time::Duration; - -use bls::{FixedBytesExtended, Keypair, Signature}; -use fork_choice::ForkChoice; -use parking_lot::{Mutex, RwLock}; -use ssz_types::VariableList; -use store::{HotColdDB, KeyValueStore, MemoryStore, StoreConfig}; -use types::consts::gloas::BUILDER_INDEX_SELF_BUILD; -use types::test_utils::generate_deterministic_keypairs; -use types::*; - -use crate::BeaconStore; -use crate::beacon_fork_choice_store::BeaconForkChoiceStore; -use crate::beacon_proposer_cache::BeaconProposerCache; -use crate::builder::Witness; -use crate::canonical_head::CanonicalHead; -use crate::payload_envelope_verification::EnvelopeError; -use crate::payload_envelope_verification::gossip_verified_envelope::{ - GossipVerificationContext, GossipVerifiedEnvelope, -}; -use crate::validator_pubkey_cache::ValidatorPubkeyCache; - -type TestEthSpec = MinimalEthSpec; -type TestTypes = Witness< - slot_clock::TestingSlotClock, - TestEthSpec, - MemoryStore, - MemoryStore, ->; - -/// Test context that holds the minimal state needed for gossip verification. -struct TestContext { - store: BeaconStore, - canonical_head: CanonicalHead, - beacon_proposer_cache: Mutex, - validator_pubkey_cache: RwLock>, - spec: Arc, - keypairs: Vec, - genesis_state: BeaconState, - genesis_block_root: Hash256, - genesis_validators_root: Hash256, -} - -impl TestContext { - fn new(validator_count: usize) -> Self { - let spec = Arc::new(ForkName::Gloas.make_genesis_spec(ChainSpec::minimal())); - let keypairs = generate_deterministic_keypairs(validator_count); - - let mut genesis_state = genesis::interop_genesis_state::( - &keypairs, - 0, // genesis_time - Hash256::from_slice(&[0x42; 32]), - None, // no execution payload header - &spec, - ) - .expect("should create genesis state"); - - let genesis_validators_root = genesis_state.genesis_validators_root(); - - let store: BeaconStore = Arc::new( - HotColdDB::open_ephemeral(StoreConfig::default(), spec.clone()) - .expect("should create ephemeral store"), - ); - - // Initialize store metadata. - let genesis_block = BeaconBlock::::empty(&spec); - let genesis_block_root = genesis_block.canonical_root(); - let signed_genesis_block = SignedBeaconBlock::from_block(genesis_block, Signature::empty()); - - // Build caches and compute state root before storing. - genesis_state - .build_caches(&spec) - .expect("should build caches"); - - // Initialize store metadata ops (must be done before put_state). - let ops = vec![ - store - .init_anchor_info( - signed_genesis_block.parent_root(), - signed_genesis_block.slot(), - Slot::new(0), - false, - ) - .expect("should init anchor info"), - store - .init_blob_info(signed_genesis_block.slot()) - .expect("should init blob info"), - store - .init_data_column_info(signed_genesis_block.slot()) - .expect("should init data column info"), - ]; - store - .hot_db - .do_atomically(ops) - .expect("should store metadata"); - - // Store the genesis block and state. - store - .put_block(&genesis_block_root, signed_genesis_block.clone()) - .expect("should store genesis block"); - let state_root = genesis_state - .update_tree_hash_cache() - .expect("should compute state root"); - store - .put_state(&state_root, &genesis_state) - .expect("should store genesis state"); - - // Create BeaconSnapshot and fork choice. - let snapshot = crate::BeaconSnapshot { - beacon_block: Arc::new(signed_genesis_block), - beacon_block_root: genesis_block_root, - beacon_state: genesis_state.clone(), - }; - - let fc_store = BeaconForkChoiceStore::get_forkchoice_store(store.clone(), snapshot.clone()) - .expect("should create fork choice store"); - - let fork_choice = ForkChoice::from_anchor( - fc_store, - genesis_block_root, - &snapshot.beacon_block, - &snapshot.beacon_state, - None, - &spec, - ) - .expect("should create fork choice from anchor"); - - let canonical_head = CanonicalHead::new(fork_choice, Arc::new(snapshot)); - - let validator_pubkey_cache = ValidatorPubkeyCache::new(&genesis_state, store.clone()) - .expect("should create validator pubkey cache"); - - TestContext { - store, - canonical_head, - beacon_proposer_cache: Mutex::new(BeaconProposerCache::default()), - validator_pubkey_cache: RwLock::new(validator_pubkey_cache), - spec, - keypairs, - genesis_state, - genesis_block_root, - genesis_validators_root, - } - } - - fn gossip_verification_context(&self) -> GossipVerificationContext<'_, TestTypes> { - GossipVerificationContext { - canonical_head: &self.canonical_head, - store: &self.store, - spec: &self.spec, - beacon_proposer_cache: &self.beacon_proposer_cache, - validator_pubkey_cache: &self.validator_pubkey_cache, - genesis_validators_root: self.genesis_validators_root, - } - } - - /// Build a gloas block at `slot` with the given proposer, store it, add it to fork choice, - /// and return the signed block, block root, and post-state. - fn build_and_import_block( - &self, - slot: Slot, - proposer_index: usize, - execution_bid: ExecutionPayloadBid, - ) -> ( - Arc>, - Hash256, - BeaconState, - ) { - let mut state = self.genesis_state.clone(); - - // Advance the state to the target slot. - if slot > state.slot() { - state_processing::state_advance::complete_state_advance( - &mut state, None, slot, &self.spec, - ) - .expect("should advance state"); - } - - state.build_caches(&self.spec).expect("should build caches"); - - // Compute the state root so we can embed it in the block. - let state_root = state - .update_tree_hash_cache() - .expect("should compute state root"); - - let signed_bid = SignedExecutionPayloadBid { - message: execution_bid, - signature: Signature::infinity().expect("should create infinity signature"), - }; - - // Create the block body with the actual state root. - let block = BeaconBlock::Gloas(BeaconBlockGloas { - slot, - proposer_index: proposer_index as u64, - parent_root: self.genesis_block_root, - state_root, - body: BeaconBlockBodyGloas { - randao_reveal: Signature::empty(), - eth1_data: state.eth1_data().clone(), - graffiti: Graffiti::default(), - proposer_slashings: VariableList::empty(), - attester_slashings: VariableList::empty(), - attestations: VariableList::empty(), - deposits: VariableList::empty(), - voluntary_exits: VariableList::empty(), - sync_aggregate: SyncAggregate::empty(), - bls_to_execution_changes: VariableList::empty(), - signed_execution_payload_bid: signed_bid, - payload_attestations: VariableList::empty(), - _phantom: std::marker::PhantomData, - }, - }); - - let block_root = block.canonical_root(); - let proposer_sk = &self.keypairs[proposer_index].sk; - let fork = self - .spec - .fork_at_epoch(slot.epoch(TestEthSpec::slots_per_epoch())); - let signed_block = block.sign(proposer_sk, &fork, self.genesis_validators_root, &self.spec); - - // Store block and state. - self.store - .put_block(&block_root, signed_block.clone()) - .expect("should store block"); - self.store - .put_state(&state_root, &state) - .expect("should store state"); - - // Add block to fork choice. - let mut fork_choice = self.canonical_head.fork_choice_write_lock(); - fork_choice - .on_block( - slot, - signed_block.message(), - block_root, - Duration::from_secs(0), - &state, - crate::PayloadVerificationStatus::Verified, - &self.spec, - ) - .expect("should add block to fork choice"); - drop(fork_choice); - - (Arc::new(signed_block), block_root, state) - } - - /// Build a signed execution payload envelope for the given block. - fn build_signed_envelope( - &self, - block_root: Hash256, - slot: Slot, - builder_index: u64, - block_hash: ExecutionBlockHash, - signing_key: &bls::SecretKey, - ) -> Arc> { - let mut payload = ExecutionPayloadGloas::default(); - payload.block_hash = block_hash; - - let envelope = ExecutionPayloadEnvelope { - payload, - execution_requests: ExecutionRequests::default(), - builder_index, - beacon_block_root: block_root, - slot, - state_root: Hash256::zero(), - }; - - let fork = self - .spec - .fork_at_epoch(slot.epoch(TestEthSpec::slots_per_epoch())); - let domain = self.spec.get_domain( - slot.epoch(TestEthSpec::slots_per_epoch()), - Domain::BeaconBuilder, - &fork, - self.genesis_validators_root, - ); - let message = envelope.signing_root(domain); - let signature = signing_key.sign(message); - - Arc::new(SignedExecutionPayloadEnvelope { - message: envelope, - signature, - }) - } - - /// Helper: build a block and matching self-build envelope. - fn build_block_and_envelope( - &self, - ) -> ( - Arc>, - Hash256, - Arc>, - ) { - let slot = Slot::new(1); - let block_hash = ExecutionBlockHash::from_root(Hash256::from_slice(&[0xaa; 32])); - - // Get proposer for slot 1. - let mut state = self.genesis_state.clone(); - state_processing::state_advance::complete_state_advance(&mut state, None, slot, &self.spec) - .expect("should advance state"); - state.build_caches(&self.spec).expect("should build caches"); - let proposer_index = state - .get_beacon_proposer_index(slot, &self.spec) - .expect("should get proposer index"); - - let bid = ExecutionPayloadBid { - builder_index: BUILDER_INDEX_SELF_BUILD, - block_hash, - slot, - ..Default::default() - }; - - let (signed_block, block_root, _post_state) = - self.build_and_import_block(slot, proposer_index, bid); - - let proposer_sk = &self.keypairs[proposer_index].sk; - let envelope = self.build_signed_envelope( - block_root, - slot, - BUILDER_INDEX_SELF_BUILD, - block_hash, - proposer_sk, - ); - - (signed_block, block_root, envelope) - } -} - -#[test] -fn test_valid_self_build_envelope() { - let ctx = TestContext::new(32); - let (_block, _block_root, envelope) = ctx.build_block_and_envelope(); - let gossip_ctx = ctx.gossip_verification_context(); - - let result = GossipVerifiedEnvelope::new(envelope, &gossip_ctx); - assert!( - result.is_ok(), - "valid self-build envelope should pass verification, got: {:?}", - result.err() - ); -} - -#[test] -fn test_unknown_block_root() { - let ctx = TestContext::new(32); - let gossip_ctx = ctx.gossip_verification_context(); - - // Build an envelope referencing a block root not in fork choice. - let unknown_root = Hash256::from_slice(&[0xff; 32]); - let envelope = ctx.build_signed_envelope( - unknown_root, - Slot::new(1), - BUILDER_INDEX_SELF_BUILD, - ExecutionBlockHash::from_root(Hash256::zero()), - &ctx.keypairs[0].sk, - ); - - let result = GossipVerifiedEnvelope::new(envelope, &gossip_ctx); - assert!( - matches!(result, Err(EnvelopeError::BlockRootUnknown { .. })), - "should reject envelope with unknown block root, got: {:?}", - result - ); -} - -#[test] -fn test_slot_mismatch() { - let ctx = TestContext::new(32); - let (_block, block_root, _good_envelope) = ctx.build_block_and_envelope(); - let gossip_ctx = ctx.gossip_verification_context(); - - // Build an envelope with a different slot than the block. - let wrong_slot = Slot::new(2); - let envelope = ctx.build_signed_envelope( - block_root, - wrong_slot, - BUILDER_INDEX_SELF_BUILD, - ExecutionBlockHash::from_root(Hash256::from_slice(&[0xaa; 32])), - &ctx.keypairs[0].sk, - ); - - let result = GossipVerifiedEnvelope::new(envelope, &gossip_ctx); - assert!( - matches!(result, Err(EnvelopeError::SlotMismatch { .. })), - "should reject envelope with slot mismatch, got: {:?}", - result - ); -} - -#[test] -fn test_builder_index_mismatch() { - let ctx = TestContext::new(32); - let gossip_ctx = ctx.gossip_verification_context(); - - let slot = Slot::new(1); - let block_hash = ExecutionBlockHash::from_root(Hash256::from_slice(&[0xaa; 32])); - - // Get proposer for slot 1. - let mut state = ctx.genesis_state.clone(); - state_processing::state_advance::complete_state_advance(&mut state, None, slot, &ctx.spec) - .expect("should advance state"); - state.build_caches(&ctx.spec).expect("should build caches"); - let proposer_index = state - .get_beacon_proposer_index(slot, &ctx.spec) - .expect("should get proposer index"); - - // Block has builder_index = BUILDER_INDEX_SELF_BUILD - let bid = ExecutionPayloadBid { - builder_index: BUILDER_INDEX_SELF_BUILD, - block_hash, - slot, - ..Default::default() - }; - let (_block, block_root, _post_state) = ctx.build_and_import_block(slot, proposer_index, bid); - - // Envelope has a different builder_index. - let wrong_builder_index = 999; - let envelope = ctx.build_signed_envelope( - block_root, - slot, - wrong_builder_index, - block_hash, - &ctx.keypairs[proposer_index].sk, - ); - - let result = GossipVerifiedEnvelope::new(envelope, &gossip_ctx); - assert!( - matches!(result, Err(EnvelopeError::BuilderIndexMismatch { .. })), - "should reject envelope with builder index mismatch, got: {:?}", - result - ); -} - -#[test] -fn test_block_hash_mismatch() { - let ctx = TestContext::new(32); - let gossip_ctx = ctx.gossip_verification_context(); - - let slot = Slot::new(1); - let block_hash = ExecutionBlockHash::from_root(Hash256::from_slice(&[0xaa; 32])); - let wrong_block_hash = ExecutionBlockHash::from_root(Hash256::from_slice(&[0xbb; 32])); - - // Get proposer for slot 1. - let mut state = ctx.genesis_state.clone(); - state_processing::state_advance::complete_state_advance(&mut state, None, slot, &ctx.spec) - .expect("should advance state"); - state.build_caches(&ctx.spec).expect("should build caches"); - let proposer_index = state - .get_beacon_proposer_index(slot, &ctx.spec) - .expect("should get proposer index"); - - // Block has block_hash = 0xaa - let bid = ExecutionPayloadBid { - builder_index: BUILDER_INDEX_SELF_BUILD, - block_hash, - slot, - ..Default::default() - }; - let (_block, block_root, _post_state) = ctx.build_and_import_block(slot, proposer_index, bid); - - // Envelope has a different block_hash. - let envelope = ctx.build_signed_envelope( - block_root, - slot, - BUILDER_INDEX_SELF_BUILD, - wrong_block_hash, - &ctx.keypairs[proposer_index].sk, - ); - - let result = GossipVerifiedEnvelope::new(envelope, &gossip_ctx); - assert!( - matches!(result, Err(EnvelopeError::BlockHashMismatch { .. })), - "should reject envelope with block hash mismatch, got: {:?}", - result - ); -} - -#[test] -fn test_bad_signature() { - let ctx = TestContext::new(32); - let gossip_ctx = ctx.gossip_verification_context(); - - let slot = Slot::new(1); - let block_hash = ExecutionBlockHash::from_root(Hash256::from_slice(&[0xaa; 32])); - - // Get proposer for slot 1. - let mut state = ctx.genesis_state.clone(); - state_processing::state_advance::complete_state_advance(&mut state, None, slot, &ctx.spec) - .expect("should advance state"); - state.build_caches(&ctx.spec).expect("should build caches"); - let proposer_index = state - .get_beacon_proposer_index(slot, &ctx.spec) - .expect("should get proposer index"); - - let bid = ExecutionPayloadBid { - builder_index: BUILDER_INDEX_SELF_BUILD, - block_hash, - slot, - ..Default::default() - }; - let (_block, block_root, _post_state) = ctx.build_and_import_block(slot, proposer_index, bid); - - // Sign the envelope with the wrong key (some other validator's key). - let wrong_key_index = if proposer_index == 0 { 1 } else { 0 }; - let envelope = ctx.build_signed_envelope( - block_root, - slot, - BUILDER_INDEX_SELF_BUILD, - block_hash, - &ctx.keypairs[wrong_key_index].sk, - ); - - let result = GossipVerifiedEnvelope::new(envelope, &gossip_ctx); - assert!( - matches!(result, Err(EnvelopeError::BadSignature)), - "should reject envelope with bad signature, got: {:?}", - result - ); -} - -// NOTE: `test_prior_to_finalization` is omitted here because advancing finalization requires -// attestation-based justification which needs the full `BeaconChainHarness`. The -// `PriorToFinalization` code path is tested in the integration tests. From 6e89ba63be49ff34e4ab70a9ba82b2aee4841f98 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Tue, 24 Feb 2026 10:51:09 -0800 Subject: [PATCH 12/19] Added slot to logs --- beacon_node/beacon_chain/src/execution_payload.rs | 12 ++++-------- .../payload_notifier.rs | 2 +- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index b2d00a530a..cf6c5d83b4 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -118,6 +118,7 @@ impl PayloadNotifier { notify_new_payload( &self.chain, self.block.message().tree_hash_root(), + self.block.message().slot(), self.block.message().try_into()?, ) .await @@ -137,6 +138,7 @@ impl PayloadNotifier { pub async fn notify_new_payload( chain: &Arc>, beacon_block_root: Hash256, + slot: Slot, new_payload_request: NewPayloadRequest<'_, T::EthSpec>, ) -> Result { let execution_layer = chain @@ -163,11 +165,8 @@ pub async fn notify_new_payload( ?validation_error, ?latest_valid_hash, ?execution_block_hash, - // TODO(gloas) are these other logs important? root = ?beacon_block_root, - // graffiti = block.body().graffiti().as_utf8_lossy(), - // proposer_index = block.proposer_index(), - // slot = %block.slot(), + %slot, method = "new_payload", "Invalid execution payload" ); @@ -209,11 +208,8 @@ pub async fn notify_new_payload( warn!( ?validation_error, ?execution_block_hash, - // TODO(gloas) are these other logs important? root = ?beacon_block_root, - // graffiti = block.body().graffiti().as_utf8_lossy(), - // proposer_index = block.proposer_index(), - // slot = %block.slot(), + %slot, method = "new_payload", "Invalid execution payload block hash" ); diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs index 5b1f332b5a..a468bc5bc4 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs @@ -62,7 +62,7 @@ impl PayloadNotifier { } else { let block_root = self.envelope.message.beacon_block_root; let request = Self::build_new_payload_request(&self.envelope, &self.block)?; - notify_new_payload(&self.chain, block_root, request).await + notify_new_payload(&self.chain, block_root, self.envelope.slot(), request).await } } From 147f2e22e02eadbbec21d532543ae5293086a9d3 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Tue, 24 Feb 2026 10:59:03 -0800 Subject: [PATCH 13/19] use cached head and drop fork choice read lock earlier --- .../gossip_verified_envelope.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs index 9d555e8ad2..0c2ae6dd56 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs @@ -118,13 +118,15 @@ impl GossipVerifiedEnvelope { }); }; - let latest_finalized_slot = fork_choice_read_lock + drop(fork_choice_read_lock); + + let latest_finalized_slot = ctx + .canonical_head + .cached_head() .finalized_checkpoint() .epoch .start_slot(T::EthSpec::slots_per_epoch()); - drop(fork_choice_read_lock); - // TODO(EIP-7732): check that we haven't seen another valid `SignedExecutionPayloadEnvelope` // for this block root from this builder - envelope status table check let block = match ctx.store.try_get_full_block(&beacon_block_root)? { From fc7d6c9d24fc9a59c528b37295b0c2a763172514 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Tue, 24 Feb 2026 11:20:07 -0800 Subject: [PATCH 14/19] Add an additional defensive expected proposer check --- .../gossip_verified_envelope.rs | 15 +++++++++++++-- .../src/payload_envelope_verification/mod.rs | 5 +++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs index 0c2ae6dd56..877dcc2c2b 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs @@ -106,6 +106,9 @@ impl GossipVerifiedEnvelope { // Check that we've seen the beacon block for this envelope and that it passes validation. // TODO(EIP-7732): We might need some type of status table in order to differentiate between: + // If we have a block_processing_table, we could have a Processed(Bid, bool) state that is only + // entered post adding to fork choice. That way, we could potentially need only a single call to make + // sure the block is valid and to do all consequent checks with the bid // // 1. Blocks we haven't seen (IGNORE), and // 2. Blocks we've seen that are invalid (REJECT). @@ -147,7 +150,7 @@ impl GossipVerifiedEnvelope { // Verify the envelope signature. // - // For self-build envelopes, we can use the proposer cache for the fork and the + // For self-built envelopes, we can use the proposer cache for the fork and the // validator pubkey cache for the proposer's pubkey, avoiding a state load from disk. // For external builder envelopes, we must load the state to access the builder registry. let builder_index = envelope.builder_index; @@ -157,7 +160,7 @@ impl GossipVerifiedEnvelope { proto_block.proposer_shuffling_root_for_child_block(block_epoch, ctx.spec); let (signature_is_valid, opt_snapshot) = if builder_index == BUILDER_INDEX_SELF_BUILD { - // Fast path: self-build envelopes can be verified without loading the state. + // Fast path: self-built envelopes can be verified without loading the state. let envelope_ref = signed_envelope.as_ref(); let mut opt_snapshot = None; let proposer = beacon_proposer_cache::with_proposer_cache( @@ -176,8 +179,16 @@ impl GossipVerifiedEnvelope { Ok::<_, EnvelopeError>((snapshot.state_root, snapshot.pre_state)) }, )?; + let expected_proposer = proposer.index; let fork = proposer.fork; + if block.message().proposer_index() != expected_proposer as u64 { + return Err(EnvelopeError::IncorrectBlockProposer { + block: block.message().proposer_index(), + local_shuffling: expected_proposer as u64, + }); + } + let pubkey_cache = ctx.validator_pubkey_cache.read(); let pubkey = pubkey_cache .get(block.message().proposer_index() as usize) diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs index ae5dbfccc0..9caff959d0 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs @@ -206,6 +206,11 @@ pub enum EnvelopeError { committed_bid: ExecutionBlockHash, envelope: ExecutionBlockHash, }, + // The block's proposer_index does not match the locally computed proposer + IncorrectBlockProposer { + block: u64, + local_shuffling: u64, + }, // The slot belongs to a block that is from a slot prior than // the most recently finalized slot PriorToFinalization { From 30241f54c4883653bf99166907ea4523a99abfb7 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Tue, 24 Feb 2026 11:35:01 -0800 Subject: [PATCH 15/19] add load_snapshot_from_state_root that can be used when we've already aquired a --- .../gossip_verified_envelope.rs | 15 +++-- .../src/payload_envelope_verification/mod.rs | 60 +++++++++++-------- 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs index 877dcc2c2b..68d6e8605e 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs @@ -22,7 +22,7 @@ use crate::{ payload_envelope_verification::{ EnvelopeError, EnvelopeImportData, EnvelopeProcessingSnapshot, ExecutionPendingEnvelope, IntoExecutionPendingEnvelope, MaybeAvailableEnvelope, load_snapshot, - payload_notifier::PayloadNotifier, + load_snapshot_from_state_root, payload_notifier::PayloadNotifier, }, validator_pubkey_cache::ValidatorPubkeyCache, }; @@ -161,7 +161,6 @@ impl GossipVerifiedEnvelope { let (signature_is_valid, opt_snapshot) = if builder_index == BUILDER_INDEX_SELF_BUILD { // Fast path: self-built envelopes can be verified without loading the state. - let envelope_ref = signed_envelope.as_ref(); let mut opt_snapshot = None; let proposer = beacon_proposer_cache::with_proposer_cache( ctx.beacon_proposer_cache, @@ -174,7 +173,11 @@ impl GossipVerifiedEnvelope { %beacon_block_root, "Proposer shuffling cache miss for envelope verification" ); - let snapshot = load_snapshot(envelope_ref, ctx.canonical_head, ctx.store)?; + let snapshot = load_snapshot_from_state_root::( + beacon_block_root, + proto_block.state_root, + ctx.store, + )?; opt_snapshot = Some(Box::new(snapshot.clone())); Ok::<_, EnvelopeError>((snapshot.state_root, snapshot.pre_state)) }, @@ -205,7 +208,11 @@ impl GossipVerifiedEnvelope { } else { // TODO(gloas) if we implement a builder pubkey cache, we'll need to use it here. // External builder: must load the state to get the builder pubkey. - let snapshot = load_snapshot(signed_envelope.as_ref(), ctx.canonical_head, ctx.store)?; + let snapshot = load_snapshot_from_state_root::( + beacon_block_root, + proto_block.state_root, + ctx.store, + )?; let is_valid = signed_envelope.verify_signature_with_state(&snapshot.pre_state, ctx.spec)?; (is_valid, Some(Box::new(snapshot))) diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs index 9caff959d0..5e88d62ec1 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs @@ -291,35 +291,16 @@ impl From for EnvelopeError { } #[allow(clippy::type_complexity)] -#[instrument(skip_all, level = "debug", fields(beacon_block_root = %envelope.beacon_block_root()))] -pub(crate) fn load_snapshot( - envelope: &SignedExecutionPayloadEnvelope, - canonical_head: &CanonicalHead, +#[instrument(skip_all, level = "debug", fields(beacon_block_root = %beacon_block_root))] +/// Load state from store given a known state root and block root. +/// Use this when the proto block has already been looked up from fork choice. +pub(crate) fn load_snapshot_from_state_root( + beacon_block_root: Hash256, + block_state_root: Hash256, store: &BeaconStore, ) -> Result, EnvelopeError> { - // Reject any envelope if its block is not known to fork choice. - // - // A block that is not in fork choice is either: - // - // - Not yet imported: we should reject this envelope because we should only import it after its parent block - // has been fully imported. - // - Pre-finalized: if the parent block is _prior_ to finalization, we should ignore the envelope - // because it will revert finalization. Note that the finalized block is stored in fork - // choice, so we will not reject any child of the finalized block (this is relevant during - // genesis). - - let fork_choice_read_lock = canonical_head.fork_choice_read_lock(); - let beacon_block_root = envelope.beacon_block_root(); - let Some(proto_beacon_block) = fork_choice_read_lock.get_block(&beacon_block_root) else { - return Err(EnvelopeError::BlockRootUnknown { - block_root: beacon_block_root, - }); - }; - drop(fork_choice_read_lock); - // TODO(EIP-7732): add metrics here - let block_state_root = proto_beacon_block.state_root; // We can use `get_hot_state` here rather than `get_advanced_hot_state` because the envelope // must be from the same slot as its block (so no advance is required). let cache_state = true; @@ -339,6 +320,35 @@ pub(crate) fn load_snapshot( }) } +#[instrument(skip_all, level = "debug", fields(beacon_block_root = %envelope.beacon_block_root()))] +pub(crate) fn load_snapshot( + envelope: &SignedExecutionPayloadEnvelope, + canonical_head: &CanonicalHead, + store: &BeaconStore, +) -> Result, EnvelopeError> { + // Reject any envelope if its block is not known to fork choice. + // + // A block that is not in fork choice is either: + // + // - Not yet imported: we should reject this envelope because we should only import it after + // its parent block has been fully imported. + // - Pre-finalized: if the parent block is _prior_ to finalization, we should ignore the + // envelope because it will revert finalization. Note that the finalized block is stored in + // fork choice, so we will not reject any child of the finalized block (this is relevant + // during genesis). + + let fork_choice_read_lock = canonical_head.fork_choice_read_lock(); + let beacon_block_root = envelope.beacon_block_root(); + let Some(proto_beacon_block) = fork_choice_read_lock.get_block(&beacon_block_root) else { + return Err(EnvelopeError::BlockRootUnknown { + block_root: beacon_block_root, + }); + }; + drop(fork_choice_read_lock); + + load_snapshot_from_state_root::(beacon_block_root, proto_beacon_block.state_root, store) +} + impl IntoExecutionPendingEnvelope for Arc> { From 2093dc1f39c213d136cee90845bd0fe6911bb5b4 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Tue, 24 Feb 2026 12:08:07 -0800 Subject: [PATCH 16/19] move execution pending envelolpe logic to its own file --- .../execution_pending_envelope.rs | 140 ++++++++++++++++++ .../gossip_verified_envelope.rs | 100 +------------ .../src/payload_envelope_verification/mod.rs | 40 +---- 3 files changed, 146 insertions(+), 134 deletions(-) create mode 100644 beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs new file mode 100644 index 0000000000..dbd7478568 --- /dev/null +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs @@ -0,0 +1,140 @@ +use std::sync::Arc; + +use slot_clock::SlotClock; +use state_processing::{ + VerifySignatures, + envelope_processing::{VerifyStateRoot, process_execution_payload_envelope}, +}; +use types::{EthSpec, SignedExecutionPayloadEnvelope}; + +use crate::{ + BeaconChain, BeaconChainError, BeaconChainTypes, NotifyExecutionLayer, + PayloadVerificationOutcome, + block_verification::PayloadVerificationHandle, + payload_envelope_verification::{ + EnvelopeError, EnvelopeImportData, MaybeAvailableEnvelope, + gossip_verified_envelope::GossipVerifiedEnvelope, load_snapshot, + payload_notifier::PayloadNotifier, + }, +}; + +pub trait IntoExecutionPendingEnvelope: Sized { + fn into_execution_pending_envelope( + self, + chain: &Arc>, + notify_execution_layer: NotifyExecutionLayer, + ) -> Result, EnvelopeError>; + + fn envelope(&self) -> &Arc>; +} + +pub struct ExecutionPendingEnvelope { + pub signed_envelope: MaybeAvailableEnvelope, + pub import_data: EnvelopeImportData, + pub payload_verification_handle: PayloadVerificationHandle, +} + +impl IntoExecutionPendingEnvelope for GossipVerifiedEnvelope { + fn into_execution_pending_envelope( + self, + chain: &Arc>, + notify_execution_layer: NotifyExecutionLayer, + ) -> Result, EnvelopeError> { + let signed_envelope = self.signed_envelope; + let envelope = &signed_envelope.message; + let payload = &envelope.payload; + + // Verify the execution payload is valid + let payload_notifier = PayloadNotifier::new( + chain.clone(), + signed_envelope.clone(), + self.block.clone(), + notify_execution_layer, + )?; + let block_root = envelope.beacon_block_root; + let slot = self.block.slot(); + + let payload_verification_future = async move { + let chain = payload_notifier.chain.clone(); + if let Some(started_execution) = chain.slot_clock.now_duration() { + chain + .envelope_times_cache + .write() + .set_time_started_execution(block_root, slot, started_execution); + } + + let payload_verification_status = payload_notifier.notify_new_payload().await?; + Ok(PayloadVerificationOutcome { + payload_verification_status, + // This fork is after the merge so it'll never be the merge transition block + is_valid_merge_transition_block: false, + }) + }; + // Spawn the payload verification future as a new task, but don't wait for it to complete. + // The `payload_verification_future` will be awaited later to ensure verification completed + // successfully. + let payload_verification_handle = chain + .task_executor + .spawn_handle( + payload_verification_future, + "execution_payload_verification", + ) + .ok_or(BeaconChainError::RuntimeShutdown)?; + + let snapshot = if let Some(snapshot) = self.snapshot { + *snapshot + } else { + load_snapshot( + signed_envelope.as_ref(), + &chain.canonical_head, + &chain.store, + )? + }; + let mut state = snapshot.pre_state; + + // All the state modifications are done in envelope_processing + process_execution_payload_envelope( + &mut state, + Some(snapshot.state_root), + &signed_envelope, + // verify signature already done for GossipVerifiedEnvelope + VerifySignatures::False, + VerifyStateRoot::True, + &chain.spec, + )?; + + Ok(ExecutionPendingEnvelope { + signed_envelope: MaybeAvailableEnvelope::AvailabilityPending { + block_hash: payload.block_hash, + envelope: signed_envelope, + }, + import_data: EnvelopeImportData { + block_root, + block: self.block, + post_state: Box::new(state), + }, + payload_verification_handle, + }) + } + + fn envelope(&self) -> &Arc> { + &self.signed_envelope + } +} + +impl IntoExecutionPendingEnvelope + for Arc> +{ + fn into_execution_pending_envelope( + self, + chain: &Arc>, + notify_execution_layer: NotifyExecutionLayer, + ) -> Result, EnvelopeError> { + GossipVerifiedEnvelope::new(self, &chain.gossip_verification_context())? + .into_execution_pending_envelope(chain, notify_execution_layer) + } + + fn envelope(&self) -> &Arc> { + self + } +} diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs index 68d6e8605e..8c8ee57fb4 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs @@ -2,11 +2,6 @@ use std::sync::Arc; use educe::Educe; use parking_lot::{Mutex, RwLock}; -use slot_clock::SlotClock; -use state_processing::{ - VerifySignatures, - envelope_processing::{VerifyStateRoot, process_execution_payload_envelope}, -}; use store::DatabaseBlock; use tracing::{Span, debug}; use types::{ @@ -15,14 +10,11 @@ use types::{ }; use crate::{ - BeaconChain, BeaconChainError, BeaconChainTypes, BeaconStore, NotifyExecutionLayer, - PayloadVerificationOutcome, + BeaconChain, BeaconChainError, BeaconChainTypes, BeaconStore, beacon_proposer_cache::{self, BeaconProposerCache}, canonical_head::CanonicalHead, payload_envelope_verification::{ - EnvelopeError, EnvelopeImportData, EnvelopeProcessingSnapshot, ExecutionPendingEnvelope, - IntoExecutionPendingEnvelope, MaybeAvailableEnvelope, load_snapshot, - load_snapshot_from_state_root, payload_notifier::PayloadNotifier, + EnvelopeError, EnvelopeProcessingSnapshot, load_snapshot_from_state_root, }, validator_pubkey_cache::ValidatorPubkeyCache, }; @@ -234,94 +226,6 @@ impl GossipVerifiedEnvelope { } } -impl IntoExecutionPendingEnvelope for GossipVerifiedEnvelope { - fn into_execution_pending_envelope( - self, - chain: &Arc>, - notify_execution_layer: NotifyExecutionLayer, - ) -> Result, EnvelopeError> { - let signed_envelope = self.signed_envelope; - let envelope = &signed_envelope.message; - let payload = &envelope.payload; - - // Verify the execution payload is valid - let payload_notifier = PayloadNotifier::new( - chain.clone(), - signed_envelope.clone(), - self.block.clone(), - notify_execution_layer, - )?; - let block_root = envelope.beacon_block_root; - let slot = self.block.slot(); - - let payload_verification_future = async move { - let chain = payload_notifier.chain.clone(); - if let Some(started_execution) = chain.slot_clock.now_duration() { - chain - .envelope_times_cache - .write() - .set_time_started_execution(block_root, slot, started_execution); - } - - let payload_verification_status = payload_notifier.notify_new_payload().await?; - Ok(PayloadVerificationOutcome { - payload_verification_status, - // This fork is after the merge so it'll never be the merge transition block - is_valid_merge_transition_block: false, - }) - }; - // Spawn the payload verification future as a new task, but don't wait for it to complete. - // The `payload_verification_future` will be awaited later to ensure verification completed - // successfully. - let payload_verification_handle = chain - .task_executor - .spawn_handle( - payload_verification_future, - "execution_payload_verification", - ) - .ok_or(BeaconChainError::RuntimeShutdown)?; - - let snapshot = if let Some(snapshot) = self.snapshot { - *snapshot - } else { - load_snapshot( - signed_envelope.as_ref(), - &chain.canonical_head, - &chain.store, - )? - }; - let mut state = snapshot.pre_state; - - // All the state modifications are done in envelope_processing - process_execution_payload_envelope( - &mut state, - Some(snapshot.state_root), - &signed_envelope, - // verify signature already done for GossipVerifiedEnvelope - VerifySignatures::False, - VerifyStateRoot::True, - &chain.spec, - )?; - - Ok(ExecutionPendingEnvelope { - signed_envelope: MaybeAvailableEnvelope::AvailabilityPending { - block_hash: payload.block_hash, - envelope: signed_envelope, - }, - import_data: EnvelopeImportData { - block_root, - block: self.block, - post_state: Box::new(state), - }, - payload_verification_handle, - }) - } - - fn envelope(&self) -> &Arc> { - &self.signed_envelope - } -} - impl BeaconChain { /// Build a `GossipVerificationContext` from this `BeaconChain`. pub fn gossip_verification_context(&self) -> GossipVerificationContext<'_, T> { diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs index 5e88d62ec1..b5193f8e8c 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs @@ -39,31 +39,16 @@ use types::{ }; use crate::{ - BeaconChain, BeaconChainError, BeaconChainTypes, BeaconStore, BlockError, - ExecutionPayloadError, NotifyExecutionLayer, PayloadVerificationOutcome, - block_verification::PayloadVerificationHandle, canonical_head::CanonicalHead, - payload_envelope_verification::gossip_verified_envelope::GossipVerifiedEnvelope, + BeaconChainError, BeaconChainTypes, BeaconStore, BlockError, ExecutionPayloadError, + PayloadVerificationOutcome, canonical_head::CanonicalHead, }; +pub mod execution_pending_envelope; pub mod gossip_verified_envelope; pub mod import; mod payload_notifier; -pub trait IntoExecutionPendingEnvelope: Sized { - fn into_execution_pending_envelope( - self, - chain: &Arc>, - notify_execution_layer: NotifyExecutionLayer, - ) -> Result, EnvelopeError>; - - fn envelope(&self) -> &Arc>; -} - -pub struct ExecutionPendingEnvelope { - pub signed_envelope: MaybeAvailableEnvelope, - pub import_data: EnvelopeImportData, - pub payload_verification_handle: PayloadVerificationHandle, -} +pub use execution_pending_envelope::{ExecutionPendingEnvelope, IntoExecutionPendingEnvelope}; #[derive(PartialEq)] pub struct EnvelopeImportData { @@ -348,20 +333,3 @@ pub(crate) fn load_snapshot( load_snapshot_from_state_root::(beacon_block_root, proto_beacon_block.state_root, store) } - -impl IntoExecutionPendingEnvelope - for Arc> -{ - fn into_execution_pending_envelope( - self, - chain: &Arc>, - notify_execution_layer: NotifyExecutionLayer, - ) -> Result, EnvelopeError> { - GossipVerifiedEnvelope::new(self, &chain.gossip_verification_context())? - .into_execution_pending_envelope(chain, notify_execution_layer) - } - - fn envelope(&self) -> &Arc> { - self - } -} From 876e6899cd10c7a68374c63d0827dedddb5df5bc Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Tue, 24 Feb 2026 12:14:30 -0800 Subject: [PATCH 17/19] Some more TODOs --- .../execution_pending_envelope.rs | 2 ++ .../src/payload_envelope_verification/payload_notifier.rs | 1 + .../network/src/network_beacon_processor/gossip_methods.rs | 3 ++- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs index dbd7478568..eea50d9fe1 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs @@ -44,6 +44,8 @@ impl IntoExecutionPendingEnvelope for GossipVerifiedEnve let envelope = &signed_envelope.message; let payload = &envelope.payload; + // TODO(gloas) + // Verify the execution payload is valid let payload_notifier = PayloadNotifier::new( chain.clone(), diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs index a468bc5bc4..592d46022a 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs @@ -29,6 +29,7 @@ impl PayloadNotifier { let payload_verification_status = { let payload_message = &envelope.message; + // TODO(gloas) re-asses if optimistic syncing works similarly post-gloas match notify_execution_layer { NotifyExecutionLayer::No if chain.config.optimistic_finalized_sync => { let new_payload_request = Self::build_new_payload_request(&envelope, &block)?; diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index 89563c2ec3..f8636f5429 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -3272,7 +3272,8 @@ impl NetworkBeaconProcessor { Span::current().record("beacon_block_root", beacon_block_root.to_string()); // TODO(gloas) in process_gossip_block here we check_and_insert on the duplicate cache - // before calling gossip_verified_block + // before calling gossip_verified_block. We need this to ensure we dont try to execute the + // payload multiple times. self.process_gossip_verified_execution_payload_envelope( peer_id, From 0761da770df8383d54bdcb37866e5805d92e3065 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Tue, 24 Feb 2026 12:15:52 -0800 Subject: [PATCH 18/19] Clean up comments --- .../src/payload_envelope_verification/import.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs index 603e14446a..4ed9bc973b 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -241,11 +241,7 @@ impl BeaconChain { _payload_verification_status: PayloadVerificationStatus, parent_block: Arc>, ) -> Result { - // ----------------------------- ENVELOPE NOT YET ATTESTABLE ---------------------------------- - // Everything in this initial section is on the hot path between processing the envelope and - // being able to attest to it. DO NOT add any extra processing in this initial section - // unless it must run before fork choice. - // ----------------------------------------------------------------------------------------- + // Everything in this initial section is on the hot path for processing the envelope. let post_exec_timer = metrics::start_timer(&metrics::ENVELOPE_PROCESSING_POST_EXEC_PROCESSING); @@ -276,12 +272,10 @@ impl BeaconChain { // TODO(gloas) emit SSE event if the payload became the new head payload drop(post_exec_timer); - // ---------------------------- ENVELOPE PROBABLY ATTESTABLE ---------------------------------- // It is important NOT to return errors here before the database commit, because the envelope // has already been added to fork choice and the database would be left in an inconsistent // state if we returned early without committing. In other words, an error here would // corrupt the node's database permanently. - // ----------------------------------------------------------------------------------------- // Store the envelope and its state, and execute the confirmation batch for the intermediate // states, which will delete their temporary flags. From 38ef0d07e548c4eb6d6597e3f3bd713828b35f6f Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Tue, 24 Feb 2026 12:17:12 -0800 Subject: [PATCH 19/19] Update TODO --- .../beacon_chain/src/payload_envelope_verification/import.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs index 4ed9bc973b..05a18a6d18 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -28,6 +28,7 @@ impl BeaconChain { /// Items that implement `IntoExecutionPendingEnvelope` include: /// /// - `GossipVerifiedEnvelope` + /// - TODO(gloas) implement for envelopes recieved over RPC /// /// ## Errors ///