From fd0852a8e59be905824cebfb4f2fe028a920159b Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 19 May 2026 11:35:31 +1000 Subject: [PATCH 1/7] Remove outdated SPRP hint (#9312) While working on this code in another branch I noticed we had this messy, complicated and incorrect code about SPRP (slots-per-restore-point), which is no longer a relevant concept since the introduction of hot state diffs. In the name of simplicity, I've removed any kind of hinting here in favour of a simple out of bounds error. The benefit of adding complex hinting code (which is not tested) to such a function is not worth it IMO. Users will work it out (or ask) if we just tell them their request is out of bounds. Co-Authored-By: Michael Sproul --- beacon_node/http_api/src/beacon/states.rs | 74 +++++++++-------------- 1 file changed, 27 insertions(+), 47 deletions(-) diff --git a/beacon_node/http_api/src/beacon/states.rs b/beacon_node/http_api/src/beacon/states.rs index 84ef3c1f26..52b05a807b 100644 --- a/beacon_node/http_api/src/beacon/states.rs +++ b/beacon_node/http_api/src/beacon/states.rs @@ -390,54 +390,34 @@ pub fn get_beacon_state_committees( if let Some(shuffling) = maybe_cached_shuffling { shuffling } else { - let possibly_built_cache = - match RelativeEpoch::from_epoch(current_epoch, epoch) { - Ok(relative_epoch) - if state.committee_cache_is_initialized( - relative_epoch, - ) => - { - state.committee_cache(relative_epoch).cloned() - } - _ => CommitteeCache::initialized( - state, - epoch, - &chain.spec, - ), + let possibly_built_cache = match RelativeEpoch::from_epoch( + current_epoch, + epoch, + ) { + Ok(relative_epoch) + if state.committee_cache_is_initialized( + relative_epoch, + ) => + { + state.committee_cache(relative_epoch).cloned() } - .map_err( - |e| match e { - BeaconStateError::EpochOutOfBounds => { - let max_sprp = - T::EthSpec::slots_per_historical_root() - as u64; - let first_subsequent_restore_point_slot = - ((epoch.start_slot( - T::EthSpec::slots_per_epoch(), - ) / max_sprp) - + 1) - * max_sprp; - if epoch < current_epoch { - warp_utils::reject::custom_bad_request( - format!( - "epoch out of bounds, \ - try state at slot {}", - first_subsequent_restore_point_slot, - ), - ) - } else { - warp_utils::reject::custom_bad_request( - "epoch out of bounds, \ - too far in future" - .into(), - ) - } - } - _ => warp_utils::reject::unhandled_error( - BeaconChainError::from(e), - ), - }, - )?; + _ => CommitteeCache::initialized( + state, + epoch, + &chain.spec, + ), + } + .map_err(|e| match e { + BeaconStateError::EpochOutOfBounds => { + warp_utils::reject::custom_bad_request(format!( + "epoch {} out of bounds for state at {}", + epoch, current_epoch + )) + } + _ => warp_utils::reject::unhandled_error( + BeaconChainError::from(e), + ), + })?; // Attempt to write to the beacon cache (only if the cache // size is not the default value). From 398efc3acca5c8d01befbbe09d35d24cbd04752c Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Mon, 18 May 2026 23:12:17 -0600 Subject: [PATCH 2/7] Use dedicated cache for HTTP API route (#9318) - PR https://github.com/sigp/lighthouse/pull/9305 wants to store PTCs in the committee cache. BUT the http API route wants to use the committee cache and insert historical committees (i.e. given state at epoch 1000, compute and store the committee for epoch 900). If we want a single cache to serve both use cases we need to: - Have entries in the committee cache that have no PTC: Makes reading PTCs from the cache not deterministic - Compute historical PTC: A bunch of complicated code that's useless Instead we can add a separate cache for the API, very simple one, that caches committees only. And have the one in the beacon chain compute and cache PTCs always. ### Performance impact Slightly additional memory cost for users of the `beacon/states/committees` route. Caching is almost equivalent, except for queries of recent committees that may already exist in the beacon chain's committee cache. ### AI disclousure This PR was written by hand 90%. Claude fixed some warp type issues Co-Authored-By: dapplion <35266934+dapplion@users.noreply.github.com> --- beacon_node/client/src/builder.rs | 5 ++ beacon_node/http_api/src/beacon/states.rs | 81 ++++++++++++----------- beacon_node/http_api/src/caches.rs | 43 ++++++++++++ beacon_node/http_api/src/lib.rs | 17 ++++- beacon_node/http_api/src/test_utils.rs | 7 +- beacon_node/src/config.rs | 3 + 6 files changed, 115 insertions(+), 41 deletions(-) create mode 100644 beacon_node/http_api/src/caches.rs diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index 9dfb8304bc..f532ef716e 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -36,6 +36,7 @@ use rand::SeedableRng; use rand::rngs::{OsRng, StdRng}; use slasher::Slasher; use slasher_service::SlasherService; +use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; @@ -639,6 +640,10 @@ where network_globals: self.network_globals.clone(), beacon_processor_send: Some(beacon_processor_channels.beacon_processor_tx.clone()), sse_logging_components: runtime_context.sse_logging_components.clone(), + historical_committee_cache: Arc::new(http_api::HistoricalCommitteeCache::new( + NonZeroUsize::new(self.http_api_config.historical_committee_cache_size) + .unwrap_or(NonZeroUsize::MIN), + )), }); let exit = runtime_context.executor.exit(); diff --git a/beacon_node/http_api/src/beacon/states.rs b/beacon_node/http_api/src/beacon/states.rs index 52b05a807b..1b765aa227 100644 --- a/beacon_node/http_api/src/beacon/states.rs +++ b/beacon_node/http_api/src/beacon/states.rs @@ -1,4 +1,5 @@ use crate::StateId; +use crate::caches::{HistoricalCommitteeCache, HistoricalShufflingId}; use crate::task_spawner::{Priority, TaskSpawner}; use crate::utils::ResponseFilter; use crate::validator::pubkey_to_validator_index; @@ -13,7 +14,10 @@ use eth2::types::{ }; use ssz::Encode; use std::sync::Arc; -use types::{AttestationShufflingId, BeaconStateError, CommitteeCache, EthSpec, RelativeEpoch}; +use types::{ + AttestationShufflingId, BeaconStateError, CommitteeCache, EthSpec, RelativeEpoch, + RelativeEpochError, +}; use warp::filters::BoxedFilter; use warp::http::Response; use warp::hyper::Body; @@ -26,6 +30,8 @@ type BeaconStatesPath = BoxedFilter<( Arc>, )>; +type BeaconStatesCommitteesFilter = BoxedFilter<(Arc,)>; + // GET beacon/states/{state_id}/pending_consolidations pub fn get_beacon_state_pending_consolidations( beacon_states_path: BeaconStatesPath, @@ -337,17 +343,20 @@ pub fn get_beacon_state_sync_committees( // GET beacon/states/{state_id}/committees?slot,index,epoch pub fn get_beacon_state_committees( beacon_states_path: BeaconStatesPath, + beacon_states_committees_filter: BeaconStatesCommitteesFilter, ) -> ResponseFilter { beacon_states_path .clone() .and(warp::path("committees")) .and(warp::query::()) + .and(beacon_states_committees_filter) .and(warp::path::end()) .then( |state_id: StateId, task_spawner: TaskSpawner, chain: Arc>, - query: eth2::types::CommitteesQuery| { + query: eth2::types::CommitteesQuery, + historical_committee_cache: Arc| { task_spawner.blocking_json_task(Priority::P1, move || { let (data, execution_optimistic, finalized) = state_id .map_state_and_execution_optimistic_and_finalized( @@ -364,33 +373,33 @@ pub fn get_beacon_state_committees( let shuffling_id = if let Ok(Some(shuffling_decision_block)) = chain.block_root_at_slot(decision_slot, WhenSlotSkipped::Prev) { - Some(AttestationShufflingId { - shuffling_epoch: epoch, - shuffling_decision_block, - }) + Some(HistoricalShufflingId::ShufflingId( + AttestationShufflingId { + shuffling_epoch: epoch, + shuffling_decision_block, + }, + )) + } else if epoch < chain.head().finalized_checkpoint().epoch { + // Use the case for finalized epochs + Some(HistoricalShufflingId::FinalizedEpoch(epoch)) } else { None }; // Attempt to read from the chain cache if there exists a // shuffling_id - let maybe_cached_shuffling = if let Some(shuffling_id) = - shuffling_id.as_ref() - { - chain - .shuffling_cache - .try_write_for(std::time::Duration::from_secs(1)) - .and_then(|mut cache_write| cache_write.get(shuffling_id)) - .and_then(|cache_item| cache_item.wait().ok()) - } else { - None - }; + let maybe_cached_shuffling = + if let Some(shuffling_id) = shuffling_id.as_ref() { + historical_committee_cache.get(shuffling_id) + } else { + None + }; let committee_cache = if let Some(shuffling) = maybe_cached_shuffling { shuffling } else { - let possibly_built_cache = match RelativeEpoch::from_epoch( + let committee_cache = match RelativeEpoch::from_epoch( current_epoch, epoch, ) { @@ -401,11 +410,19 @@ pub fn get_beacon_state_committees( { state.committee_cache(relative_epoch).cloned() } - _ => CommitteeCache::initialized( - state, - epoch, - &chain.spec, - ), + Ok(_) | Err(RelativeEpochError::EpochTooLow { .. }) => { + CommitteeCache::initialized( + state, + epoch, + &chain.spec, + ) + } + Err(RelativeEpochError::EpochTooHigh { .. }) => { + Err(BeaconStateError::EpochOutOfBounds) + } + Err(RelativeEpochError::ArithError(e)) => { + Err(BeaconStateError::ArithError(e)) + } } .map_err(|e| match e { BeaconStateError::EpochOutOfBounds => { @@ -419,22 +436,12 @@ pub fn get_beacon_state_committees( ), })?; - // Attempt to write to the beacon cache (only if the cache - // size is not the default value). - if chain.config.shuffling_cache_size - != beacon_chain::shuffling_cache::DEFAULT_CACHE_SIZE - && let Some(shuffling_id) = shuffling_id - && let Some(mut cache_write) = chain - .shuffling_cache - .try_write_for(std::time::Duration::from_secs(1)) - { - cache_write.insert_committee_cache( - shuffling_id, - &possibly_built_cache, - ); + if let Some(shuffling_id) = shuffling_id { + historical_committee_cache + .insert(shuffling_id, committee_cache.clone()); } - possibly_built_cache + committee_cache }; // Use either the supplied slot or all slots in the epoch. diff --git a/beacon_node/http_api/src/caches.rs b/beacon_node/http_api/src/caches.rs new file mode 100644 index 0000000000..d92571594a --- /dev/null +++ b/beacon_node/http_api/src/caches.rs @@ -0,0 +1,43 @@ +use lru::LruCache; +use parking_lot::Mutex; +use std::num::NonZeroUsize; +use std::sync::Arc; +use types::{AttestationShufflingId, CommitteeCache, Epoch}; + +/// See `shuffling_cache::DEFAULT_CACHE_SIZE` for rationale +pub const DEFAULT_HISTORICAL_COMMITTEE_CACHE_SIZE: usize = 16; + +/// Indexes the `HistoricalCommitteeCache`. We can compute committees for very old epochs, and we +/// can't retrieve the decision root cheaply from a state. For those cases we allow the cache to +/// key those committees by finalized epoch. +#[derive(Eq, Hash, PartialEq)] +pub enum HistoricalShufflingId { + FinalizedEpoch(Epoch), + ShufflingId(AttestationShufflingId), +} + +/// Dedicated cache for attestation committees, used exclusively by the HTTP API. +/// +/// This may contain committees for finalized and unfinalized epochs. The name is slightly +/// missleading :) +pub struct HistoricalCommitteeCache { + committees: Mutex>>, +} + +impl HistoricalCommitteeCache { + pub fn new(size: NonZeroUsize) -> Self { + Self { + committees: Mutex::new(LruCache::new(size)), + } + } +} + +impl HistoricalCommitteeCache { + pub fn get(&self, id: &HistoricalShufflingId) -> Option> { + self.committees.lock().get(id).cloned() + } + + pub fn insert(&self, id: HistoricalShufflingId, cache: Arc) { + self.committees.lock().put(id, cache); + } +} diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index f31817c5ba..74bf1ccd76 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -12,6 +12,7 @@ mod beacon; mod block_id; mod build_block_contents; mod builder_states; +mod caches; mod custody; mod database; mod light_client; @@ -40,6 +41,8 @@ use crate::beacon::execution_payload_envelope::{ post_beacon_execution_payload_envelope_ssz, }; use crate::beacon::pool::*; +use crate::caches::DEFAULT_HISTORICAL_COMMITTEE_CACHE_SIZE; +pub use crate::caches::HistoricalCommitteeCache; use crate::light_client::{get_light_client_bootstrap, get_light_client_updates}; use crate::utils::{AnyVersionFilter, EthV1Filter}; use crate::validator::post_validator_liveness_epoch; @@ -132,6 +135,7 @@ pub struct Context { pub network_globals: Option>>, pub beacon_processor_send: Option>, pub sse_logging_components: Option, + pub historical_committee_cache: Arc, } /// Configuration for the HTTP server. @@ -148,6 +152,7 @@ pub struct Config { #[serde(with = "eth2::types::serde_status_code")] pub duplicate_block_status_code: StatusCode, pub target_peers: usize, + pub historical_committee_cache_size: usize, } impl Default for Config { @@ -163,6 +168,7 @@ impl Default for Config { enable_beacon_processor: true, duplicate_block_status_code: StatusCode::ACCEPTED, target_peers: 100, + historical_committee_cache_size: DEFAULT_HISTORICAL_COMMITTEE_CACHE_SIZE, } } } @@ -416,6 +422,11 @@ pub fn serve( }) .boxed(); + let historical_committee_cache = ctx.historical_committee_cache.clone(); + let beacon_states_committees_filter = warp::any() + .map(move || historical_committee_cache.clone()) + .boxed(); + // Create a `warp` filter that provides access to the network sender channel. let network_tx = ctx .network_senders @@ -628,8 +639,10 @@ pub fn serve( states::get_beacon_state_validators_id(beacon_states_path.clone()); // GET beacon/states/{state_id}/committees?slot,index,epoch - let get_beacon_state_committees = - states::get_beacon_state_committees(beacon_states_path.clone()); + let get_beacon_state_committees = states::get_beacon_state_committees( + beacon_states_path.clone(), + beacon_states_committees_filter, + ); // GET beacon/states/{state_id}/sync_committees?epoch let get_beacon_state_sync_committees = diff --git a/beacon_node/http_api/src/test_utils.rs b/beacon_node/http_api/src/test_utils.rs index 27e2a27d35..f27a04d17a 100644 --- a/beacon_node/http_api/src/test_utils.rs +++ b/beacon_node/http_api/src/test_utils.rs @@ -1,4 +1,4 @@ -use crate::{Config, Context}; +use crate::{Config, Context, caches::HistoricalCommitteeCache}; use beacon_chain::{ BeaconChain, BeaconChainTypes, custody_context::NodeCustodyType, @@ -22,10 +22,10 @@ use lighthouse_network::{ }; use network::{NetworkReceivers, NetworkSenders}; use sensitive_url::SensitiveUrl; -use std::future::Future; use std::net::SocketAddr; use std::sync::Arc; use std::time::Duration; +use std::{future::Future, num::NonZeroUsize}; use store::MemoryStore; use task_executor::test_utils::TestRuntime; use types::{ChainSpec, EthSpec}; @@ -293,6 +293,9 @@ pub async fn create_api_server_with_config( network_globals: Some(network_globals), beacon_processor_send: Some(beacon_processor_send), sse_logging_components: None, + historical_committee_cache: Arc::new(HistoricalCommitteeCache::new( + NonZeroUsize::new(http_config.historical_committee_cache_size).unwrap(), + )), }); let (listening_socket, server) = diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 8ba2c0f321..f10f9e3b45 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -215,6 +215,9 @@ pub fn get_config( if let Some(cache_size) = clap_utils::parse_optional(cli_args, "shuffling-cache-size")? { client_config.chain.shuffling_cache_size = cache_size; + // Mantain backwards compatibility with users customizing `shuffling_cache_size` to tweak + // the behaviour of the HTTP API route `beacon/states/committees` + client_config.http_api.historical_committee_cache_size = cache_size; } if let Some(batches) = clap_utils::parse_optional(cli_args, "blob-publication-batches")? { From 2c76ee5b6b03cdcd43563e89d1befa7f07f4cc75 Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Wed, 20 May 2026 06:56:49 -0600 Subject: [PATCH 3/7] Gloas lookup sync boilerplate (#9322) Implements the boring boilerplate to send envelopes by root requests and process them. Pre-step to - https://github.com/sigp/lighthouse/pull/9155 Co-Authored-By: dapplion <35266934+dapplion@users.noreply.github.com> --- beacon_node/beacon_chain/src/beacon_chain.rs | 6 + beacon_node/beacon_processor/src/lib.rs | 10 ++ .../src/scheduler/work_queue.rs | 6 + .../src/service/api_types.rs | 2 + .../src/network_beacon_processor/mod.rs | 19 +++ .../network_beacon_processor/sync_methods.rs | 57 +++++++ beacon_node/network/src/router.rs | 35 ++++- .../network/src/sync/block_lookups/mod.rs | 2 + beacon_node/network/src/sync/manager.rs | 66 +++++++- .../network/src/sync/network_context.rs | 145 +++++++++++++++++- .../src/sync/network_context/requests.rs | 4 + .../requests/payload_envelopes_by_root.rs | 54 +++++++ 12 files changed, 398 insertions(+), 8 deletions(-) create mode 100644 beacon_node/network/src/sync/network_context/requests/payload_envelopes_by_root.rs diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index af8cd477d6..f3f6cd299e 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -6339,6 +6339,12 @@ impl BeaconChain { .contains_block(root) } + pub fn envelope_is_known_to_fork_choice(&self, root: &Hash256) -> bool { + self.canonical_head + .fork_choice_read_lock() + .is_payload_received(root) + } + /// Determines the beacon proposer for the next slot. If that proposer is registered in the /// `execution_layer`, provide the `execution_layer` with the necessary information to produce /// `PayloadAttributes` for future calls to fork choice. diff --git a/beacon_node/beacon_processor/src/lib.rs b/beacon_node/beacon_processor/src/lib.rs index 25944bcf8a..ce3851ea54 100644 --- a/beacon_node/beacon_processor/src/lib.rs +++ b/beacon_node/beacon_processor/src/lib.rs @@ -418,6 +418,7 @@ pub enum Work { process_fn: AsyncFn, }, RpcCustodyColumn(AsyncFn), + RpcEnvelope(AsyncFn), ColumnReconstruction(AsyncFn), IgnoredRpcBlock { process_fn: BlockingFn, @@ -485,6 +486,7 @@ pub enum WorkType { RpcBlock, RpcBlobs, RpcCustodyColumn, + RpcEnvelope, ColumnReconstruction, IgnoredRpcBlock, ChainSegment, @@ -548,6 +550,7 @@ impl Work { Work::RpcBlock { .. } => WorkType::RpcBlock, Work::RpcBlobs { .. } => WorkType::RpcBlobs, Work::RpcCustodyColumn { .. } => WorkType::RpcCustodyColumn, + Work::RpcEnvelope(_) => WorkType::RpcEnvelope, Work::ColumnReconstruction(_) => WorkType::ColumnReconstruction, Work::IgnoredRpcBlock { .. } => WorkType::IgnoredRpcBlock, Work::ChainSegment { .. } => WorkType::ChainSegment, @@ -825,6 +828,8 @@ impl BeaconProcessor { Some(item) } else if let Some(item) = work_queues.rpc_custody_column_queue.pop() { Some(item) + } else if let Some(item) = work_queues.rpc_envelope_queue.pop() { + Some(item) // Check delayed blocks before gossip blocks, the gossip blocks might rely // on the delayed ones. } else if let Some(item) = work_queues.delayed_block_queue.pop() { @@ -1192,6 +1197,9 @@ impl BeaconProcessor { work_queues.rpc_block_queue.push(work, work_id) } Work::RpcBlobs { .. } => work_queues.rpc_blob_queue.push(work, work_id), + Work::RpcEnvelope(_) => { + work_queues.rpc_envelope_queue.push(work, work_id) + } Work::RpcCustodyColumn { .. } => { work_queues.rpc_custody_column_queue.push(work, work_id) } @@ -1330,6 +1338,7 @@ impl BeaconProcessor { WorkType::RpcBlobs | WorkType::IgnoredRpcBlock => { work_queues.rpc_blob_queue.len() } + WorkType::RpcEnvelope => work_queues.rpc_envelope_queue.len(), WorkType::RpcCustodyColumn => work_queues.rpc_custody_column_queue.len(), WorkType::ColumnReconstruction => { work_queues.column_reconstruction_queue.len() @@ -1523,6 +1532,7 @@ impl BeaconProcessor { } | Work::RpcBlobs { process_fn } | Work::RpcCustodyColumn(process_fn) + | Work::RpcEnvelope(process_fn) | Work::ColumnReconstruction(process_fn) => task_spawner.spawn_async(process_fn), Work::IgnoredRpcBlock { process_fn } => task_spawner.spawn_blocking(process_fn), Work::GossipBlock(work) diff --git a/beacon_node/beacon_processor/src/scheduler/work_queue.rs b/beacon_node/beacon_processor/src/scheduler/work_queue.rs index eb57b97df2..2fdc15182c 100644 --- a/beacon_node/beacon_processor/src/scheduler/work_queue.rs +++ b/beacon_node/beacon_processor/src/scheduler/work_queue.rs @@ -120,6 +120,7 @@ pub struct BeaconProcessorQueueLengths { rpc_block_queue: usize, rpc_blob_queue: usize, rpc_custody_column_queue: usize, + rpc_envelope_queue: usize, column_reconstruction_queue: usize, chain_segment_queue: usize, backfill_chain_segment: usize, @@ -195,6 +196,8 @@ impl BeaconProcessorQueueLengths { // We don't request more than `PARENT_DEPTH_TOLERANCE` (32) lookups, so we can limit // this queue size. With 48 max blobs per block, each column sidecar list could be up to 12MB. rpc_custody_column_queue: 64, + // Bounded by `PARENT_DEPTH_TOLERANCE`; one envelope per Gloas block. + rpc_envelope_queue: 1024, column_reconstruction_queue: 1, chain_segment_queue: 64, backfill_chain_segment: 64, @@ -253,6 +256,7 @@ pub struct WorkQueues { pub rpc_block_queue: FifoQueue>, pub rpc_blob_queue: FifoQueue>, pub rpc_custody_column_queue: FifoQueue>, + pub rpc_envelope_queue: FifoQueue>, pub column_reconstruction_queue: LifoQueue>, pub chain_segment_queue: FifoQueue>, pub backfill_chain_segment: FifoQueue>, @@ -323,6 +327,7 @@ impl WorkQueues { let rpc_block_queue = FifoQueue::new(queue_lengths.rpc_block_queue); let rpc_blob_queue = FifoQueue::new(queue_lengths.rpc_blob_queue); let rpc_custody_column_queue = FifoQueue::new(queue_lengths.rpc_custody_column_queue); + let rpc_envelope_queue = FifoQueue::new(queue_lengths.rpc_envelope_queue); let column_reconstruction_queue = LifoQueue::new(queue_lengths.column_reconstruction_queue); let chain_segment_queue = FifoQueue::new(queue_lengths.chain_segment_queue); let backfill_chain_segment = FifoQueue::new(queue_lengths.backfill_chain_segment); @@ -391,6 +396,7 @@ impl WorkQueues { rpc_block_queue, rpc_blob_queue, rpc_custody_column_queue, + rpc_envelope_queue, chain_segment_queue, column_reconstruction_queue, backfill_chain_segment, diff --git a/beacon_node/lighthouse_network/src/service/api_types.rs b/beacon_node/lighthouse_network/src/service/api_types.rs index f598f59aee..2429b813e9 100644 --- a/beacon_node/lighthouse_network/src/service/api_types.rs +++ b/beacon_node/lighthouse_network/src/service/api_types.rs @@ -23,6 +23,8 @@ pub enum SyncRequestId { SingleBlock { id: SingleLookupReqId }, /// Request searching for a set of blobs given a hash. SingleBlob { id: SingleLookupReqId }, + /// Request searching for a payload envelope given a hash. + SinglePayloadEnvelope { id: SingleLookupReqId }, /// Request searching for a set of data columns given a hash and list of column indices. DataColumnsByRoot(DataColumnsByRootRequestId), /// Blocks by range request diff --git a/beacon_node/network/src/network_beacon_processor/mod.rs b/beacon_node/network/src/network_beacon_processor/mod.rs index 7bf969db10..7817feb0bd 100644 --- a/beacon_node/network/src/network_beacon_processor/mod.rs +++ b/beacon_node/network/src/network_beacon_processor/mod.rs @@ -588,6 +588,25 @@ impl NetworkBeaconProcessor { }) } + /// Create a new `Work` event for an RPC-fetched payload envelope. `process_lookup_envelope` + /// reports the result back to sync. + pub fn send_lookup_envelope( + self: &Arc, + block_root: Hash256, + envelope: Arc>, + seen_timestamp: Duration, + process_type: BlockProcessType, + ) -> Result<(), Error> { + let s = self.clone(); + self.try_send(BeaconWorkEvent { + drop_during_sync: false, + work: Work::RpcEnvelope(Box::pin(async move { + s.process_lookup_envelope(block_root, envelope, seen_timestamp, process_type) + .await; + })), + }) + } + /// Create a new `Work` event for some custody columns. `process_rpc_custody_columns` reports /// the result back to sync. pub fn send_rpc_custody_columns( diff --git a/beacon_node/network/src/network_beacon_processor/sync_methods.rs b/beacon_node/network/src/network_beacon_processor/sync_methods.rs index 988a68c9dd..e3ba6fb3c4 100644 --- a/beacon_node/network/src/network_beacon_processor/sync_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/sync_methods.rs @@ -426,6 +426,63 @@ impl NetworkBeaconProcessor { }); } + /// Attempt to verify and import an execution payload envelope received via RPC. + #[instrument( + name = "lh_process_lookup_envelope", + parent = None, + level = "debug", + skip_all, + fields(?block_root), + )] + pub async fn process_lookup_envelope( + self: Arc>, + block_root: Hash256, + envelope: Arc>, + _seen_timestamp: Duration, + process_type: BlockProcessType, + ) { + debug!( + ?block_root, + slot = %envelope.slot(), + ?process_type, + "Processing RPC payload envelope" + ); + + // Gossip verification runs the same signature / slot / builder-index / block-hash checks + // independently of gossip propagation, so we can reuse it for RPC-fetched envelopes. + #[allow(clippy::result_large_err)] + let result = match self + .chain + .clone() + .verify_envelope_for_gossip(envelope.clone()) + .await + { + Ok(verified) => { + self.chain + .process_execution_payload_envelope( + block_root, + verified, + NotifyExecutionLayer::Yes, + BlockImportSource::Lookup, + || Ok(()), + ) + .await + } + Err(e) => Err(e), + }; + + // TODO(gloas): structured penalty classification arrives with the envelope lookup state + // machine; for now, fold the EnvelopeError into BlockError::InternalError so it flows + // through the existing `BlockProcessingResult::Err` path. + let result: Result = + result.map_err(|e| BlockError::InternalError(format!("envelope: {e}"))); + + self.send_sync_message(SyncMessage::BlockComponentProcessed { + process_type, + result: result.into(), + }); + } + pub fn process_historic_data_columns( &self, batch_id: CustodyBackfillBatchId, diff --git a/beacon_node/network/src/router.rs b/beacon_node/network/src/router.rs index a718997e0a..35939c6f39 100644 --- a/beacon_node/network/src/router.rs +++ b/beacon_node/network/src/router.rs @@ -26,6 +26,7 @@ use tokio_stream::wrappers::UnboundedReceiverStream; use tracing::{debug, error, trace, warn}; use types::{ BlobSidecar, DataColumnSidecar, EthSpec, ForkContext, PartialDataColumn, SignedBeaconBlock, + SignedExecutionPayloadEnvelope, }; /// Handles messages from the network and routes them to the appropriate service to be handled. @@ -348,10 +349,13 @@ impl Router { Response::DataColumnsByRange(data_column) => { self.on_data_columns_by_range_response(peer_id, app_request_id, data_column); } - // TODO(EIP-7732): implement outgoing payload envelopes by range and root - // responses once sync manager requests them. - Response::PayloadEnvelopesByRoot(_) | Response::PayloadEnvelopesByRange(_) => { - debug!("Requesting envelopes by root and by range not supported yet"); + Response::PayloadEnvelopesByRoot(envelope) => { + self.on_payload_envelopes_by_root_response(peer_id, app_request_id, envelope); + } + // TODO(EIP-7732): implement outgoing payload envelopes by range responses + // once sync manager requests them. + Response::PayloadEnvelopesByRange(_) => { + debug!("Requesting envelopes by range not supported yet"); } // Lighthouse currently only serves BlocksByHead and does not issue it as a client, // so receiving a response is unexpected. Drop it without crashing. @@ -821,6 +825,29 @@ impl Router { } } + /// Handle a `PayloadEnvelopesByRoot` response from the peer. + pub fn on_payload_envelopes_by_root_response( + &mut self, + peer_id: PeerId, + app_request_id: AppRequestId, + envelope: Option>>, + ) { + let sync_request_id = match app_request_id { + AppRequestId::Sync(id @ SyncRequestId::SinglePayloadEnvelope { .. }) => id, + other => { + crit!(request = ?other, %peer_id, "PayloadEnvelopesByRoot response on incorrect request"); + return; + } + }; + + self.send_to_sync(SyncMessage::RpcPayloadEnvelope { + sync_request_id, + peer_id, + envelope, + seen_timestamp: self.chain.slot_clock.now_duration().unwrap_or_default(), + }); + } + fn handle_beacon_processor_send_result( &mut self, result: Result<(), crate::network_beacon_processor::Error>, diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index 3929f74aa0..f10610c751 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -559,6 +559,8 @@ impl BlockLookups { BlockProcessType::SingleCustodyColumn(id) => { self.on_processing_result_inner::>(id, result, cx) } + // TODO(gloas): route into the payload envelope lookup state machine. + BlockProcessType::SinglePayloadEnvelope(_) => Ok(LookupResult::Pending), }; self.on_lookup_result(process_type.id(), lookup_result, "processing_result", cx); } diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index 347b018a93..14a38f0e72 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -73,7 +73,8 @@ use strum::IntoStaticStr; use tokio::sync::mpsc; use tracing::{debug, error, info, trace}; use types::{ - BlobSidecar, DataColumnSidecar, EthSpec, ForkContext, Hash256, SignedBeaconBlock, Slot, + BlobSidecar, DataColumnSidecar, EthSpec, ForkContext, Hash256, SignedBeaconBlock, + SignedExecutionPayloadEnvelope, Slot, }; /// The number of slots ahead of us that is allowed before requesting a long-range (batch) Sync @@ -132,6 +133,14 @@ pub enum SyncMessage { seen_timestamp: Duration, }, + /// A payload envelope has been received from the RPC. + RpcPayloadEnvelope { + sync_request_id: SyncRequestId, + peer_id: PeerId, + envelope: Option>>, + seen_timestamp: Duration, + }, + /// A block with an unknown parent has been received. UnknownParentBlock(PeerId, Arc>, Hash256), @@ -193,6 +202,7 @@ pub enum BlockProcessType { SingleBlock { id: Id }, SingleBlob { id: Id }, SingleCustodyColumn(Id), + SinglePayloadEnvelope(Id), } impl BlockProcessType { @@ -200,7 +210,8 @@ impl BlockProcessType { match self { BlockProcessType::SingleBlock { id } | BlockProcessType::SingleBlob { id } - | BlockProcessType::SingleCustodyColumn(id) => *id, + | BlockProcessType::SingleCustodyColumn(id) + | BlockProcessType::SinglePayloadEnvelope(id) => *id, } } } @@ -502,6 +513,9 @@ impl SyncManager { SyncRequestId::SingleBlob { id } => { self.on_single_blob_response(id, peer_id, RpcEvent::RPCError(error)) } + SyncRequestId::SinglePayloadEnvelope { id } => { + self.on_single_payload_envelope_response(id, peer_id, RpcEvent::RPCError(error)) + } SyncRequestId::DataColumnsByRoot(req_id) => { self.on_data_columns_by_root_response(req_id, peer_id, RpcEvent::RPCError(error)) } @@ -848,6 +862,17 @@ impl SyncManager { } => { self.rpc_data_column_received(sync_request_id, peer_id, data_column, seen_timestamp) } + SyncMessage::RpcPayloadEnvelope { + sync_request_id, + peer_id, + envelope, + seen_timestamp, + } => self.rpc_payload_envelope_received( + sync_request_id, + peer_id, + envelope, + seen_timestamp, + ), SyncMessage::UnknownParentBlock(peer_id, block, block_root) => { let block_slot = block.slot(); let parent_root = block.parent_root(); @@ -1209,6 +1234,27 @@ impl SyncManager { } } + // TODO(gloas): dispatch into block_lookups once the envelope lookup state machine lands. + fn rpc_payload_envelope_received( + &mut self, + sync_request_id: SyncRequestId, + peer_id: PeerId, + envelope: Option>>, + seen_timestamp: Duration, + ) { + match sync_request_id { + SyncRequestId::SinglePayloadEnvelope { id } => self + .on_single_payload_envelope_response( + id, + peer_id, + RpcEvent::from_chunk(envelope, seen_timestamp), + ), + _ => { + crit!(%peer_id, "bad request id for payload envelope"); + } + } + } + fn rpc_data_column_received( &mut self, sync_request_id: SyncRequestId, @@ -1237,6 +1283,22 @@ impl SyncManager { } } + fn on_single_payload_envelope_response( + &mut self, + id: SingleLookupReqId, + peer_id: PeerId, + envelope: RpcEvent>>, + ) { + if let Some(_resp) = self + .network + .on_single_payload_envelope_response(id, peer_id, envelope) + { + // TODO(gloas): dispatch into + // `block_lookups.on_download_response::>(...)` once + // the envelope lookup state machine lands. + } + } + fn on_single_blob_response( &mut self, id: SingleLookupReqId, diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 465e23998b..9d5ac40c0a 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -2,7 +2,10 @@ //! channel and stores a global RPC ID to perform requests. use self::custody::{ActiveCustodyRequest, Error as CustodyRequestError}; -pub use self::requests::{BlocksByRootSingleRequest, DataColumnsByRootSingleBlockRequest}; +pub use self::requests::{ + BlocksByRootSingleRequest, DataColumnsByRootSingleBlockRequest, + PayloadEnvelopesByRootSingleRequest, +}; use super::SyncMessage; use super::block_sidecar_coupling::RangeBlockComponentsRequest; use super::manager::BlockProcessType; @@ -37,6 +40,7 @@ pub use requests::LookupVerifyError; use requests::{ ActiveRequests, BlobsByRangeRequestItems, BlobsByRootRequestItems, BlocksByRangeRequestItems, BlocksByRootRequestItems, DataColumnsByRangeRequestItems, DataColumnsByRootRequestItems, + PayloadEnvelopesByRootRequestItems, }; #[cfg(test)] use slot_clock::SlotClock; @@ -52,7 +56,7 @@ use tracing::{Span, debug, debug_span, error, warn}; use types::data::FixedBlobSidecarList; use types::{ BlobSidecar, BlockImportSource, ColumnIndex, DataColumnSidecar, DataColumnSidecarList, EthSpec, - ForkContext, Hash256, SignedBeaconBlock, Slot, + ForkContext, Hash256, SignedBeaconBlock, SignedExecutionPayloadEnvelope, Slot, }; pub mod custody; @@ -201,6 +205,9 @@ pub struct SyncNetworkContext { ActiveRequests>, /// A mapping of active BlobsByRoot requests, including both current slot and parent lookups. blobs_by_root_requests: ActiveRequests>, + /// A mapping of active PayloadEnvelopesByRoot requests + payload_envelopes_by_root_requests: + ActiveRequests>, /// A mapping of active DataColumnsByRoot requests data_columns_by_root_requests: ActiveRequests>, @@ -294,6 +301,7 @@ impl SyncNetworkContext { request_id: 1, blocks_by_root_requests: ActiveRequests::new("blocks_by_root"), blobs_by_root_requests: ActiveRequests::new("blobs_by_root"), + payload_envelopes_by_root_requests: ActiveRequests::new("payload_envelopes_by_root"), data_columns_by_root_requests: ActiveRequests::new("data_columns_by_root"), blocks_by_range_requests: ActiveRequests::new("blocks_by_range"), blobs_by_range_requests: ActiveRequests::new("blobs_by_range"), @@ -322,6 +330,7 @@ impl SyncNetworkContext { request_id: _, blocks_by_root_requests, blobs_by_root_requests, + payload_envelopes_by_root_requests, data_columns_by_root_requests, blocks_by_range_requests, blobs_by_range_requests, @@ -345,6 +354,10 @@ impl SyncNetworkContext { .active_requests_of_peer(peer_id) .into_iter() .map(|id| SyncRequestId::SingleBlob { id: *id }); + let payload_envelopes_by_root_ids = payload_envelopes_by_root_requests + .active_requests_of_peer(peer_id) + .into_iter() + .map(|id| SyncRequestId::SinglePayloadEnvelope { id: *id }); let data_column_by_root_ids = data_columns_by_root_requests .active_requests_of_peer(peer_id) .into_iter() @@ -363,6 +376,7 @@ impl SyncNetworkContext { .map(|req_id| SyncRequestId::DataColumnsByRange(*req_id)); blocks_by_root_ids .chain(blobs_by_root_ids) + .chain(payload_envelopes_by_root_ids) .chain(data_column_by_root_ids) .chain(blocks_by_range_ids) .chain(blobs_by_range_ids) @@ -419,6 +433,7 @@ impl SyncNetworkContext { request_id: _, blocks_by_root_requests, blobs_by_root_requests, + payload_envelopes_by_root_requests, data_columns_by_root_requests, blocks_by_range_requests, blobs_by_range_requests, @@ -441,6 +456,7 @@ impl SyncNetworkContext { for peer_id in blocks_by_root_requests .iter_request_peers() .chain(blobs_by_root_requests.iter_request_peers()) + .chain(payload_envelopes_by_root_requests.iter_request_peers()) .chain(data_columns_by_root_requests.iter_request_peers()) .chain(blocks_by_range_requests.iter_request_peers()) .chain(blobs_by_range_requests.iter_request_peers()) @@ -927,6 +943,81 @@ impl SyncNetworkContext { Ok(LookupRequestResult::RequestSent(id.req_id)) } + /// Request a payload envelope for a block root via PayloadEnvelopesByRoot RPC. + #[allow(dead_code)] + pub fn payload_lookup_request( + &mut self, + lookup_id: SingleLookupId, + lookup_peers: Arc>>, + block_root: Hash256, + ) -> Result { + // Skip the download if fork-choice already saw this envelope (e.g. imported via gossip + // before the lookup got here). + if self.chain.envelope_is_known_to_fork_choice(&block_root) { + return Ok(LookupRequestResult::NoRequestNeeded( + "envelope already known to fork-choice", + )); + } + + let active_request_count_by_peer = self.active_request_count_by_peer(); + let Some(peer_id) = lookup_peers + .read() + .iter() + .map(|peer| { + ( + active_request_count_by_peer.get(peer).copied().unwrap_or(0), + rand::random::(), + peer, + ) + }) + .min() + .map(|(_, _, peer)| *peer) + else { + return Ok(LookupRequestResult::Pending("no peers")); + }; + + let id = SingleLookupReqId { + lookup_id, + req_id: self.next_id(), + }; + + let request = PayloadEnvelopesByRootSingleRequest { block_root }; + + let network_request = RequestType::PayloadEnvelopesByRoot( + request + .clone() + .into_request(&self.fork_context) + .map_err(RpcRequestSendError::InternalError)?, + ); + self.network_send + .send(NetworkMessage::SendRequest { + peer_id, + request: network_request, + app_request_id: AppRequestId::Sync(SyncRequestId::SinglePayloadEnvelope { id }), + }) + .map_err(|_| RpcRequestSendError::InternalError("network send error".to_owned()))?; + + debug!( + method = "PayloadEnvelopesByRoot", + ?block_root, + peer = %peer_id, + %id, + "Sync RPC request sent" + ); + + self.payload_envelopes_by_root_requests.insert( + id, + peer_id, + // true = enforce that the peer returns a response. We only request a single envelope + // and the peer must have it. + true, + PayloadEnvelopesByRootRequestItems::new(request), + Span::none(), + ); + + Ok(LookupRequestResult::RequestSent(id.req_id)) + } + /// Request necessary blobs for `block_root`. Requests only the necessary blobs by checking: /// - If we have a downloaded but not yet processed block /// - If the da_checker has a pending block @@ -1476,6 +1567,27 @@ impl SyncNetworkContext { self.on_rpc_response_result(resp, peer_id) } + pub(crate) fn on_single_payload_envelope_response( + &mut self, + id: SingleLookupReqId, + peer_id: PeerId, + rpc_event: RpcEvent>>, + ) -> Option>>> { + let resp = self + .payload_envelopes_by_root_requests + .on_response(id, rpc_event); + let resp = resp.map(|res| { + res.and_then(|(mut envelopes, seen_timestamp)| { + match envelopes.pop() { + Some(envelope) => Ok((envelope, seen_timestamp)), + // Should never happen, we enforce at least 1 chunk. + None => Err(LookupVerifyError::NotEnoughResponsesReturned { actual: 0 }.into()), + } + }) + }); + self.on_rpc_response_result(resp, peer_id) + } + #[allow(clippy::type_complexity)] pub(crate) fn on_data_columns_by_root_response( &mut self, @@ -1652,6 +1764,35 @@ impl SyncNetworkContext { }) } + #[allow(dead_code)] + pub fn send_payload_for_processing( + &self, + block_root: Hash256, + envelope: Arc>, + seen_timestamp: Duration, + process_type: BlockProcessType, + ) -> Result<(), SendErrorProcessor> { + let beacon_processor = self + .beacon_processor_if_enabled() + .ok_or(SendErrorProcessor::ProcessorNotAvailable)?; + + debug!( + ?block_root, + ?process_type, + "Sending payload envelope for processing" + ); + + beacon_processor + .send_lookup_envelope(block_root, envelope, seen_timestamp, process_type) + .map_err(|e| { + error!( + error = ?e, + "Failed to send sync payload envelope to processor" + ); + SendErrorProcessor::SendError + }) + } + pub fn send_custody_columns_for_processing( &self, _id: Id, diff --git a/beacon_node/network/src/sync/network_context/requests.rs b/beacon_node/network/src/sync/network_context/requests.rs index ad60dffb45..8c091eca80 100644 --- a/beacon_node/network/src/sync/network_context/requests.rs +++ b/beacon_node/network/src/sync/network_context/requests.rs @@ -16,6 +16,9 @@ pub use data_columns_by_range::DataColumnsByRangeRequestItems; pub use data_columns_by_root::{ DataColumnsByRootRequestItems, DataColumnsByRootSingleBlockRequest, }; +pub use payload_envelopes_by_root::{ + PayloadEnvelopesByRootRequestItems, PayloadEnvelopesByRootSingleRequest, +}; use crate::metrics; @@ -27,6 +30,7 @@ mod blocks_by_range; mod blocks_by_root; mod data_columns_by_range; mod data_columns_by_root; +mod payload_envelopes_by_root; #[derive(Debug, PartialEq, Eq, IntoStaticStr)] pub enum LookupVerifyError { diff --git a/beacon_node/network/src/sync/network_context/requests/payload_envelopes_by_root.rs b/beacon_node/network/src/sync/network_context/requests/payload_envelopes_by_root.rs new file mode 100644 index 0000000000..a142d86e90 --- /dev/null +++ b/beacon_node/network/src/sync/network_context/requests/payload_envelopes_by_root.rs @@ -0,0 +1,54 @@ +use lighthouse_network::rpc::methods::PayloadEnvelopesByRootRequest; +use std::sync::Arc; +use types::{EthSpec, ForkContext, Hash256, SignedExecutionPayloadEnvelope}; + +use super::{ActiveRequestItems, LookupVerifyError}; + +#[derive(Debug, Clone)] +pub struct PayloadEnvelopesByRootSingleRequest { + pub block_root: Hash256, +} + +impl PayloadEnvelopesByRootSingleRequest { + pub fn into_request( + self, + fork_context: &ForkContext, + ) -> Result { + PayloadEnvelopesByRootRequest::new(vec![self.block_root], fork_context) + } +} + +pub struct PayloadEnvelopesByRootRequestItems { + request: PayloadEnvelopesByRootSingleRequest, + items: Vec>>, +} + +impl PayloadEnvelopesByRootRequestItems { + pub fn new(request: PayloadEnvelopesByRootSingleRequest) -> Self { + Self { + request, + items: vec![], + } + } +} + +impl ActiveRequestItems for PayloadEnvelopesByRootRequestItems { + type Item = Arc>; + + /// Append a response to the single chunk request. We expect exactly one envelope per + /// block root. Returns `true` when the single expected item has been received. + fn add(&mut self, envelope: Self::Item) -> Result { + let block_root = envelope.message.beacon_block_root; + if self.request.block_root != block_root { + return Err(LookupVerifyError::UnrequestedBlockRoot(block_root)); + } + + self.items.push(envelope); + // Always returns true, we expect a single envelope per block root + Ok(true) + } + + fn consume(&mut self) -> Vec { + std::mem::take(&mut self.items) + } +} From a9637c16502abb0215b9a76b7067207b6bc70d8c Mon Sep 17 00:00:00 2001 From: Daniel Knopik <107140945+dknopik@users.noreply.github.com> Date: Thu, 21 May 2026 05:25:02 +0200 Subject: [PATCH 4/7] Partial columns cleanup (#9321) #8314 left a few ugly potentially panicking location behind - all of them believed to be unreachable, but this PR fixes them regardless for good hygiene. Update to `ethereum_ssz 0.10.4` for two new helpers: `not_inplace` and `clone_zeroed`. Remove remaining `expect` and `todo!` in favour of these helpers and one new fallible (but practically infallible) method. Co-Authored-By: Daniel Knopik --- Cargo.lock | 4 ++-- .../src/data_column_verification.rs | 11 ++++++++- beacon_node/http_api/src/publish_blocks.rs | 12 ++++++---- .../lighthouse_network/src/types/partial.rs | 18 +++++--------- .../gossip_methods.rs | 24 +++++++++++-------- .../types/src/data/data_column_sidecar.rs | 17 ++++++------- 6 files changed, 47 insertions(+), 39 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 078f699f3c..d42bcd8fc1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3282,9 +3282,9 @@ dependencies = [ [[package]] name = "ethereum_ssz" -version = "0.10.3" +version = "0.10.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "368a4a4e4273b0135111fe9464e35465067766a8f664615b5a86338b73864407" +checksum = "e462875ad8693755ea8913d6e905715c76ea4836e2254e18c9cf0f7a8f8c2a13" dependencies = [ "alloy-primitives", "arbitrary", diff --git a/beacon_node/beacon_chain/src/data_column_verification.rs b/beacon_node/beacon_chain/src/data_column_verification.rs index 71562b376b..45cd687b36 100644 --- a/beacon_node/beacon_chain/src/data_column_verification.rs +++ b/beacon_node/beacon_chain/src/data_column_verification.rs @@ -1220,7 +1220,16 @@ pub fn validate_partial_data_column_sidecar_for_gossip( header, }; } - Err(MissingCellsError::UnexpectedError(e)) => todo!("handle unexpected error {:?}", e), + Err(MissingCellsError::UnexpectedError(e)) => { + return PartialColumnVerificationResult::ErrWithValidHeader { + err: GossipDataColumnError::InternalError(format!( + "An unexpected error occurred while validating partial data columns: {:?}", + e + )) + .into(), + header, + }; + } }; // We do not have to check block related data here, as we create the verifiable column from diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index e96c86b17f..ca4ab85524 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -524,11 +524,15 @@ pub(crate) fn publish_column_sidecars( if chain.config.enable_partial_columns && let DataColumnSidecar::Fulu(fulu_data_col) = data_col.as_ref() { - let mut partial = fulu_data_col.to_partial(); - if let Some(header) = partial.sidecar.header.take() { - partial_header = Some(header); + match fulu_data_col.to_partial() { + Ok(mut partial) => { + if let Some(header) = partial.sidecar.header.take() { + partial_header = Some(header); + } + partial_columns.push(Arc::new(partial)); + } + Err(err) => crit!(?err, "Could not convert from full to partial"), } - partial_columns.push(Arc::new(partial)); } let subnet = DataColumnSubnetId::from_column_index(*data_col.index(), &chain.spec); diff --git a/beacon_node/lighthouse_network/src/types/partial.rs b/beacon_node/lighthouse_network/src/types/partial.rs index f25ce9ec36..26705b7106 100644 --- a/beacon_node/lighthouse_network/src/types/partial.rs +++ b/beacon_node/lighthouse_network/src/types/partial.rs @@ -9,7 +9,7 @@ use std::sync::Arc; use tracing::{debug, error}; use types::core::{EthSpec, Hash256}; use types::data::{ - CellBitmap, PartialDataColumn, PartialDataColumnHeader, PartialDataColumnPartsMetadata, + PartialDataColumn, PartialDataColumnHeader, PartialDataColumnPartsMetadata, PartialDataColumnSidecar, PartialDataColumnSidecarRef, }; @@ -32,12 +32,8 @@ impl OutgoingPartialColumn { header_sent_set: HeaderSentSet, ) -> Self { // For now, always request all cells - let mut requests = partial_column.sidecar.cells_present_bitmap.clone(); - for idx in 0..requests.len() { - requests - .set(idx, true) - .expect("Bound asserted via `len` above"); - } + let mut requests = partial_column.sidecar.cells_present_bitmap.clone_zeroed(); + requests.not_inplace(); let metadata = PartialDataColumnPartsMetadata:: { available: partial_column.sidecar.cells_present_bitmap.clone(), requests, @@ -45,10 +41,7 @@ impl OutgoingPartialColumn { .into(); let header_message = PartialDataColumnSidecarRef { - cells_present_bitmap: CellBitmap::::with_capacity( - partial_column.sidecar.cells_present_bitmap.len(), - ) - .expect("Taking length from bitmap with same bound"), + cells_present_bitmap: partial_column.sidecar.cells_present_bitmap.clone_zeroed(), column: vec![], kzg_proofs: vec![], header: Some(header).into(), @@ -210,7 +203,7 @@ impl Partial for OutgoingPartialColumn { let send = self .partial_column .sidecar - .filter(|idx| want.get(idx).expect("Bound checked above")) + .filter(|idx| want.get(idx).unwrap_or(false)) .map_err(|err| { error!(?err, "Unexpected error filtering sidecar"); PartialError::InvalidFormat @@ -262,6 +255,7 @@ mod tests { use fixed_bytes::FixedBytesExtended; use libp2p::identity::Keypair; use ssz_types::FixedVector; + use types::CellBitmap; use types::block::{BeaconBlockHeader, SignedBeaconBlockHeader}; use types::core::{MinimalEthSpec, Slot}; use types::data::PartialDataColumnHeader; 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 d34668b138..7a902649cb 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -1381,16 +1381,20 @@ impl NetworkBeaconProcessor { &[&data_column_index.to_string()], ); - let mut column = col.to_partial(); - let header = column.sidecar.header.take(); - if let Some(header) = header { - self.send_network_message(NetworkMessage::PublishPartialColumns { - columns: vec![Arc::new(column)], - header: Arc::new(header), - }); - } else { - crit!("Converting from full to partial yielded headerless partial") - }; + match col.to_partial() { + Ok(mut column) => { + let header = column.sidecar.header.take(); + if let Some(header) = header { + self.send_network_message(NetworkMessage::PublishPartialColumns { + columns: vec![Arc::new(column)], + header: Arc::new(header), + }); + } else { + crit!("Converting from full to partial yielded headerless partial") + }; + } + Err(err) => crit!(?err, "Could not convert from full to partial"), + } } let result = self diff --git a/consensus/types/src/data/data_column_sidecar.rs b/consensus/types/src/data/data_column_sidecar.rs index 170aa99666..d15651730f 100644 --- a/consensus/types/src/data/data_column_sidecar.rs +++ b/consensus/types/src/data/data_column_sidecar.rs @@ -250,19 +250,16 @@ impl DataColumnSidecarFulu { } /// Convert this full data column into a verifiable partial data column. - pub fn to_partial(&self) -> PartialDataColumn { + /// Note: This is not expected to ever fail. + pub fn to_partial(&self) -> Result, PartialDataColumnSidecarError> { let cell_count = self.column.len(); - let mut bitmap = - CellBitmap::::with_capacity(cell_count).expect("our column has the same bound"); - for idx in 0..cell_count { - bitmap - .set(idx, true) - .expect("The correct size is initialized right above"); - } + let mut bitmap = CellBitmap::::with_capacity(cell_count) + .map_err(|_| PartialDataColumnSidecarError::UnexpectedBounds)?; + bitmap.not_inplace(); let block_root = self.block_root(); - PartialDataColumn { + Ok(PartialDataColumn { block_root, index: self.index, sidecar: PartialDataColumnSidecar { @@ -276,7 +273,7 @@ impl DataColumnSidecarFulu { }) .into(), }, - } + }) } } From 1caaa10fa86cfe9ad47cffc03f7de81b3e6642e6 Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 21 May 2026 02:35:35 -0600 Subject: [PATCH 5/7] Drop unused EthSpec generic from Stores (#9281) Co-Authored-By: dapplion <35266934+dapplion@users.noreply.github.com> --- beacon_node/beacon_chain/src/beacon_chain.rs | 4 +- .../src/beacon_fork_choice_store.rs | 12 ++-- beacon_node/beacon_chain/src/builder.rs | 19 +++---- .../overflow_lru_cache.rs | 10 ++-- beacon_node/beacon_chain/src/migrate.rs | 4 +- .../payload_attestation_verification/tests.rs | 2 +- .../beacon_chain/src/persisted_custody.rs | 4 +- beacon_node/beacon_chain/src/test_utils.rs | 18 +++--- .../beacon_chain/tests/op_verification.rs | 2 +- .../beacon_chain/tests/prepare_payload.rs | 8 +-- .../beacon_chain/tests/schema_stability.rs | 2 +- beacon_node/beacon_chain/tests/store_tests.rs | 14 ++--- beacon_node/client/src/builder.rs | 15 +++-- beacon_node/http_api/src/test_utils.rs | 2 +- .../src/network_beacon_processor/mod.rs | 3 +- beacon_node/network/src/persisted_dht.rs | 13 ++--- .../network/src/subnet_service/tests/mod.rs | 7 +-- beacon_node/network/src/sync/tests/mod.rs | 2 +- beacon_node/src/lib.rs | 2 +- beacon_node/store/src/database/interface.rs | 13 ++--- .../store/src/database/leveldb_impl.rs | 13 ++--- beacon_node/store/src/database/redb_impl.rs | 15 ++--- beacon_node/store/src/forwards_iter.rs | 18 +++--- beacon_node/store/src/hot_cold_store.rs | 16 +++--- beacon_node/store/src/invariants.rs | 2 +- beacon_node/store/src/iter.rs | 56 ++++++++----------- beacon_node/store/src/lib.rs | 8 +-- beacon_node/store/src/memory_store.rs | 12 ++-- beacon_node/store/src/reconstruct.rs | 4 +- consensus/fork_choice/tests/tests.rs | 2 +- database_manager/src/lib.rs | 22 ++++---- 31 files changed, 141 insertions(+), 183 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index f3f6cd299e..2259e1d809 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -325,8 +325,8 @@ pub enum StateSkipConfig { } pub trait BeaconChainTypes: Send + Sync + 'static { - type HotStore: store::ItemStore; - type ColdStore: store::ItemStore; + type HotStore: store::ItemStore; + type ColdStore: store::ItemStore; type SlotClock: slot_clock::SlotClock; type EthSpec: types::EthSpec; } diff --git a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs index 95fde28f5b..133eaa2fc6 100644 --- a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs +++ b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs @@ -129,8 +129,8 @@ impl BalancesCache { /// Implements `fork_choice::ForkChoiceStore` in order to provide a persistent backing to the /// `fork_choice::ForkChoice` struct. #[derive(Debug, Educe)] -#[educe(PartialEq(bound(E: EthSpec, Hot: ItemStore, Cold: ItemStore)))] -pub struct BeaconForkChoiceStore, Cold: ItemStore> { +#[educe(PartialEq(bound(E: EthSpec, Hot: ItemStore, Cold: ItemStore)))] +pub struct BeaconForkChoiceStore { #[educe(PartialEq(ignore))] store: Arc>, balances_cache: BalancesCache, @@ -150,8 +150,8 @@ pub struct BeaconForkChoiceStore, Cold: ItemStore< impl BeaconForkChoiceStore where E: EthSpec, - Hot: ItemStore, - Cold: ItemStore, + Hot: ItemStore, + Cold: ItemStore, { /// Initialize `Self` from some `anchor` checkpoint which may or may not be the genesis state. /// @@ -267,8 +267,8 @@ where impl ForkChoiceStore for BeaconForkChoiceStore where E: EthSpec, - Hot: ItemStore, - Cold: ItemStore, + Hot: ItemStore, + Cold: ItemStore, { type Error = Error; diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index e668bef7c0..61c026e0a9 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -60,8 +60,8 @@ pub struct Witness( impl BeaconChainTypes for Witness where - THotStore: ItemStore + 'static, - TColdStore: ItemStore + 'static, + THotStore: ItemStore + 'static, + TColdStore: ItemStore + 'static, TSlotClock: SlotClock + 'static, E: EthSpec + 'static, { @@ -115,8 +115,8 @@ pub struct BeaconChainBuilder { impl BeaconChainBuilder> where - THotStore: ItemStore + 'static, - TColdStore: ItemStore + 'static, + THotStore: ItemStore + 'static, + TColdStore: ItemStore + 'static, TSlotClock: SlotClock + 'static, E: EthSpec + 'static, { @@ -1162,8 +1162,8 @@ where impl BeaconChainBuilder> where - THotStore: ItemStore + 'static, - TColdStore: ItemStore + 'static, + THotStore: ItemStore + 'static, + TColdStore: ItemStore + 'static, E: EthSpec + 'static, { /// Sets the `BeaconChain` slot clock to `TestingSlotClock`. @@ -1301,11 +1301,8 @@ mod test { let validator_count = 1; let genesis_time = 13_371_337; - let store: HotColdDB< - MinimalEthSpec, - MemoryStore, - MemoryStore, - > = HotColdDB::open_ephemeral(StoreConfig::default(), ChainSpec::minimal().into()).unwrap(); + let store: HotColdDB = + HotColdDB::open_ephemeral(StoreConfig::default(), ChainSpec::minimal().into()).unwrap(); let spec = MinimalEthSpec::default_spec(); let genesis_state = interop_genesis_state( diff --git a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs index 3034e196b9..8a80f835ab 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs @@ -802,7 +802,7 @@ mod test { fn get_store_with_spec( db_path: &TempDir, spec: Arc, - ) -> Arc, BeaconNodeBackend>> { + ) -> Arc> { let hot_path = db_path.path().join("hot_db"); let cold_path = db_path.path().join("cold_db"); let blobs_path = db_path.path().join("blobs_db"); @@ -860,8 +860,8 @@ mod test { ) where E: EthSpec, - Hot: ItemStore, - Cold: ItemStore, + Hot: ItemStore, + Cold: ItemStore, { let chain = &harness.chain; let head = chain.head_snapshot(); @@ -946,8 +946,8 @@ mod test { where E: EthSpec, T: BeaconChainTypes< - HotStore = BeaconNodeBackend, - ColdStore = BeaconNodeBackend, + HotStore = BeaconNodeBackend, + ColdStore = BeaconNodeBackend, EthSpec = E, >, { diff --git a/beacon_node/beacon_chain/src/migrate.rs b/beacon_node/beacon_chain/src/migrate.rs index 3c17c1ebba..9c70bcafa2 100644 --- a/beacon_node/beacon_chain/src/migrate.rs +++ b/beacon_node/beacon_chain/src/migrate.rs @@ -30,7 +30,7 @@ pub const DEFAULT_EPOCHS_PER_MIGRATION: u64 = 1; /// The background migrator runs a thread to perform pruning and migrate state from the hot /// to the cold database. -pub struct BackgroundMigrator, Cold: ItemStore> { +pub struct BackgroundMigrator { db: Arc>, /// Record of when the last migration ran, for enforcing `epochs_per_migration`. prev_migration: Arc>, @@ -135,7 +135,7 @@ pub struct FinalizationNotification { pub prev_migration: Arc>, } -impl, Cold: ItemStore> BackgroundMigrator { +impl BackgroundMigrator { /// Create a new `BackgroundMigrator` and spawn its thread if necessary. pub fn new(db: Arc>, config: MigratorConfig) -> Self { // Estimate last migration run from DB split slot. diff --git a/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs b/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs index 7faad98e55..c45df51ac8 100644 --- a/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs +++ b/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs @@ -43,7 +43,7 @@ struct TestContext { keypairs: Vec, spec: ChainSpec, genesis_block_root: Hash256, - store: Arc, store::MemoryStore>>, + store: Arc>, } impl TestContext { diff --git a/beacon_node/beacon_chain/src/persisted_custody.rs b/beacon_node/beacon_chain/src/persisted_custody.rs index ba221c67b5..cc7219fa90 100644 --- a/beacon_node/beacon_chain/src/persisted_custody.rs +++ b/beacon_node/beacon_chain/src/persisted_custody.rs @@ -9,7 +9,7 @@ pub const CUSTODY_DB_KEY: Hash256 = Hash256::ZERO; pub struct PersistedCustody(pub CustodyContextSsz); -pub fn load_custody_context, Cold: ItemStore>( +pub fn load_custody_context( store: Arc>, ) -> Option { let res: Result, _> = @@ -22,7 +22,7 @@ pub fn load_custody_context, Cold: ItemStore>( } /// Attempt to persist the custody context object to `self.store`. -pub fn persist_custody_context, Cold: ItemStore>( +pub fn persist_custody_context( store: Arc>, custody_context: CustodyContextSsz, ) -> Result<(), store::Error> { diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 8e9cc61208..c2ccad7d8c 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -124,8 +124,8 @@ pub fn get_kzg(spec: &ChainSpec) -> Arc { pub type BaseHarnessType = Witness; -pub type DiskHarnessType = BaseHarnessType, BeaconNodeBackend>; -pub type EphemeralHarnessType = BaseHarnessType, MemoryStore>; +pub type DiskHarnessType = BaseHarnessType; +pub type EphemeralHarnessType = BaseHarnessType; pub type BoxedMutator = Box< dyn FnOnce( @@ -334,7 +334,7 @@ impl Builder> { /// Manually restore from a given `MemoryStore`. pub fn resumed_ephemeral_store( mut self, - store: Arc, MemoryStore>>, + store: Arc>, ) -> Self { let mutator = move |builder: BeaconChainBuilder<_>| { builder @@ -350,7 +350,7 @@ impl Builder> { /// Disk store, start from genesis. pub fn fresh_disk_store( mut self, - store: Arc, BeaconNodeBackend>>, + store: Arc>, ) -> Self { let validator_keypairs = self .validator_keypairs @@ -384,7 +384,7 @@ impl Builder> { /// Disk store, resume. pub fn resumed_disk_store( mut self, - store: Arc, BeaconNodeBackend>>, + store: Arc>, ) -> Self { let mutator = move |builder: BeaconChainBuilder<_>| { builder @@ -399,8 +399,8 @@ impl Builder> { impl Builder> where E: EthSpec, - Hot: ItemStore, - Cold: ItemStore, + Hot: ItemStore, + Cold: ItemStore, { pub fn new(eth_spec_instance: E) -> Self { let runtime = TestRuntime::default(); @@ -760,8 +760,8 @@ pub type HarnessSyncContributions = Vec<( impl BeaconChainHarness> where E: EthSpec, - Hot: ItemStore, - Cold: ItemStore, + Hot: ItemStore, + Cold: ItemStore, { pub fn builder(eth_spec_instance: E) -> Builder> { create_test_tracing_subscriber(); diff --git a/beacon_node/beacon_chain/tests/op_verification.rs b/beacon_node/beacon_chain/tests/op_verification.rs index 2f97f10745..adc14541a9 100644 --- a/beacon_node/beacon_chain/tests/op_verification.rs +++ b/beacon_node/beacon_chain/tests/op_verification.rs @@ -27,7 +27,7 @@ static KEYPAIRS: LazyLock> = type E = MinimalEthSpec; type TestHarness = BeaconChainHarness>; -type HotColdDB = store::HotColdDB, BeaconNodeBackend>; +type HotColdDB = store::HotColdDB; fn get_store(db_path: &TempDir) -> Arc { let spec = Arc::new(test_spec::()); diff --git a/beacon_node/beacon_chain/tests/prepare_payload.rs b/beacon_node/beacon_chain/tests/prepare_payload.rs index 47dd1ef517..de8bfb3865 100644 --- a/beacon_node/beacon_chain/tests/prepare_payload.rs +++ b/beacon_node/beacon_chain/tests/prepare_payload.rs @@ -34,7 +34,7 @@ type TestHarness = BeaconChainHarness>; fn get_store( db_path: &TempDir, spec: Arc, -) -> Arc, BeaconNodeBackend>> { +) -> Arc> { let store_config = StoreConfig { prune_payloads: false, ..StoreConfig::default() @@ -46,7 +46,7 @@ fn get_store_generic( db_path: &TempDir, config: StoreConfig, spec: Arc, -) -> Arc, BeaconNodeBackend>> { +) -> Arc> { create_test_tracing_subscriber(); let hot_path = db_path.path().join("chain_db"); let cold_path = db_path.path().join("freezer_db"); @@ -64,7 +64,7 @@ fn get_store_generic( } fn get_harness( - store: Arc, BeaconNodeBackend>>, + store: Arc>, validator_count: usize, ) -> TestHarness { // Most tests expect to retain historic states, so we use this as the default. @@ -81,7 +81,7 @@ fn get_harness( } fn get_harness_generic( - store: Arc, BeaconNodeBackend>>, + store: Arc>, validator_count: usize, chain_config: ChainConfig, node_custody_type: NodeCustodyType, diff --git a/beacon_node/beacon_chain/tests/schema_stability.rs b/beacon_node/beacon_chain/tests/schema_stability.rs index 8200748ae6..899a40511d 100644 --- a/beacon_node/beacon_chain/tests/schema_stability.rs +++ b/beacon_node/beacon_chain/tests/schema_stability.rs @@ -20,7 +20,7 @@ use tempfile::{TempDir, tempdir}; use types::{ChainSpec, Hash256, MainnetEthSpec, Slot}; type E = MainnetEthSpec; -type Store = Arc, BeaconNodeBackend>>; +type Store = Arc>; type TestHarness = BeaconChainHarness>; const VALIDATOR_COUNT: usize = 32; diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 0ff9f6841d..7e50f4e5ac 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -106,7 +106,7 @@ fn get_or_reconstruct_blobs( } } -fn get_store(db_path: &TempDir) -> Arc, BeaconNodeBackend>> { +fn get_store(db_path: &TempDir) -> Arc> { let store_config = StoreConfig { prune_payloads: false, ..StoreConfig::default() @@ -118,7 +118,7 @@ fn get_store_generic( db_path: &TempDir, config: StoreConfig, spec: ChainSpec, -) -> Arc, BeaconNodeBackend>> { +) -> Arc> { create_test_tracing_subscriber(); let hot_path = db_path.path().join("chain_db"); let cold_path = db_path.path().join("freezer_db"); @@ -136,7 +136,7 @@ fn get_store_generic( } fn get_harness( - store: Arc, BeaconNodeBackend>>, + store: Arc>, validator_count: usize, ) -> TestHarness { // Most tests expect to retain historic states, so we use this as the default. @@ -153,7 +153,7 @@ fn get_harness( } fn get_harness_import_all_data_columns( - store: Arc, BeaconNodeBackend>>, + store: Arc>, validator_count: usize, ) -> TestHarness { // Most tests expect to retain historic states, so we use this as the default. @@ -171,7 +171,7 @@ fn get_harness_import_all_data_columns( } fn get_harness_generic( - store: Arc, BeaconNodeBackend>>, + store: Arc>, validator_count: usize, chain_config: ChainConfig, node_custody_type: NodeCustodyType, @@ -205,7 +205,7 @@ fn check_db_invariants(harness: &TestHarness) { } fn get_states_descendant_of_block( - store: &HotColdDB, BeaconNodeBackend>, + store: &HotColdDB, block_root: Hash256, ) -> Vec<(Hash256, Slot)> { let summaries = store.load_hot_state_summaries().unwrap(); @@ -5859,7 +5859,7 @@ async fn test_gloas_hot_state_hierarchy() { /// Check that the HotColdDB's split_slot is equal to the start slot of the last finalized epoch. fn check_split_slot( harness: &TestHarness, - store: Arc, BeaconNodeBackend>>, + store: Arc>, ) { let split_slot = store.get_split_slot(); assert_eq!( diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index f532ef716e..0a3c414632 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -98,8 +98,8 @@ impl where TSlotClock: SlotClock + Clone + 'static, E: EthSpec + 'static, - THotStore: ItemStore + 'static, - TColdStore: ItemStore + 'static, + THotStore: ItemStore + 'static, + TColdStore: ItemStore + 'static, { /// Instantiates a new, empty builder. /// @@ -815,8 +815,8 @@ impl where TSlotClock: SlotClock + Clone + 'static, E: EthSpec + 'static, - THotStore: ItemStore + 'static, - TColdStore: ItemStore + 'static, + THotStore: ItemStore + 'static, + TColdStore: ItemStore + 'static, { /// Consumes the internal `BeaconChainBuilder`, attaching the resulting `BeaconChain` to self. #[instrument(skip_all)] @@ -847,8 +847,7 @@ where } } -impl - ClientBuilder, BeaconNodeBackend>> +impl ClientBuilder> where TSlotClock: SlotClock + 'static, E: EthSpec + 'static, @@ -889,8 +888,8 @@ where impl ClientBuilder> where E: EthSpec + 'static, - THotStore: ItemStore + 'static, - TColdStore: ItemStore + 'static, + THotStore: ItemStore + 'static, + TColdStore: ItemStore + 'static, { /// Specifies that the slot clock should read the time from the computers system clock. pub fn system_time_slot_clock(mut self) -> Result { diff --git a/beacon_node/http_api/src/test_utils.rs b/beacon_node/http_api/src/test_utils.rs index f27a04d17a..467a5216b1 100644 --- a/beacon_node/http_api/src/test_utils.rs +++ b/beacon_node/http_api/src/test_utils.rs @@ -57,7 +57,7 @@ pub struct ApiServer> { type HarnessBuilder = Builder>; type Initializer = Box) -> HarnessBuilder>; -type Mutator = BoxedMutator, MemoryStore>; +type Mutator = BoxedMutator; impl InteractiveTester { pub async fn new(spec: Option, validator_count: usize) -> Self { diff --git a/beacon_node/network/src/network_beacon_processor/mod.rs b/beacon_node/network/src/network_beacon_processor/mod.rs index 7817feb0bd..434f7ecc8b 100644 --- a/beacon_node/network/src/network_beacon_processor/mod.rs +++ b/beacon_node/network/src/network_beacon_processor/mod.rs @@ -1267,8 +1267,7 @@ use { }; #[cfg(test)] -pub(crate) type TestBeaconChainType = - Witness, MemoryStore>; +pub(crate) type TestBeaconChainType = Witness; #[cfg(test)] impl NetworkBeaconProcessor> { diff --git a/beacon_node/network/src/persisted_dht.rs b/beacon_node/network/src/persisted_dht.rs index 113b3cdd32..3672f97113 100644 --- a/beacon_node/network/src/persisted_dht.rs +++ b/beacon_node/network/src/persisted_dht.rs @@ -6,7 +6,7 @@ use types::{EthSpec, Hash256}; /// 32-byte key for accessing the `DhtEnrs`. All zero because `DhtEnrs` has its own column. pub const DHT_DB_KEY: Hash256 = Hash256::ZERO; -pub fn load_dht, Cold: ItemStore>( +pub fn load_dht( store: Arc>, ) -> Vec { // Load DHT from store @@ -20,7 +20,7 @@ pub fn load_dht, Cold: ItemStore>( } /// Attempt to persist the ENR's in the DHT to `self.store`. -pub fn persist_dht, Cold: ItemStore>( +pub fn persist_dht( store: Arc>, enrs: Vec, ) -> Result<(), store::Error> { @@ -28,7 +28,7 @@ pub fn persist_dht, Cold: ItemStore>( } /// Attempts to clear any DHT entries. -pub fn clear_dht, Cold: ItemStore>( +pub fn clear_dht( store: Arc>, ) -> Result<(), store::Error> { store.hot_db.delete::(&DHT_DB_KEY) @@ -75,11 +75,8 @@ mod tests { use types::{ChainSpec, MinimalEthSpec}; #[test] fn test_persisted_dht() { - let store: HotColdDB< - MinimalEthSpec, - MemoryStore, - MemoryStore, - > = HotColdDB::open_ephemeral(StoreConfig::default(), ChainSpec::minimal().into()).unwrap(); + let store: HotColdDB = + HotColdDB::open_ephemeral(StoreConfig::default(), ChainSpec::minimal().into()).unwrap(); let enrs = vec![Enr::from_str("enr:-IS4QHCYrYZbAKWCBRlAy5zzaDZXJBGkcnh4MHcBFZntXNFrdvJjX04jRzjzCBOonrkTfj499SZuOh8R33Ls8RRcy5wBgmlkgnY0gmlwhH8AAAGJc2VjcDI1NmsxoQPKY0yuDUmstAHYpMa2_oxVtw0RW_QAdpzBQA8yWM0xOIN1ZHCCdl8").unwrap()]; store .put_item(&DHT_DB_KEY, &PersistedDht { enrs: enrs.clone() }) diff --git a/beacon_node/network/src/subnet_service/tests/mod.rs b/beacon_node/network/src/subnet_service/tests/mod.rs index 619154d738..745934053a 100644 --- a/beacon_node/network/src/subnet_service/tests/mod.rs +++ b/beacon_node/network/src/subnet_service/tests/mod.rs @@ -25,12 +25,7 @@ const SLOT_DURATION_MILLIS: u64 = 400; const TEST_LOG_LEVEL: Option<&str> = None; -type TestBeaconChainType = Witness< - SystemTimeSlotClock, - MainnetEthSpec, - MemoryStore, - MemoryStore, ->; +type TestBeaconChainType = Witness; pub struct TestBeaconChain { chain: Arc>, diff --git a/beacon_node/network/src/sync/tests/mod.rs b/beacon_node/network/src/sync/tests/mod.rs index dd8c3ae432..4e185cc081 100644 --- a/beacon_node/network/src/sync/tests/mod.rs +++ b/beacon_node/network/src/sync/tests/mod.rs @@ -26,7 +26,7 @@ use types::{ForkName, Hash256, MinimalEthSpec as E, Slot}; mod lookups; mod range; -type T = Witness, MemoryStore>; +type T = Witness; /// This test utility enables integration testing of Lighthouse sync components. /// diff --git a/beacon_node/src/lib.rs b/beacon_node/src/lib.rs index e33da17e26..6400427f8c 100644 --- a/beacon_node/src/lib.rs +++ b/beacon_node/src/lib.rs @@ -20,7 +20,7 @@ use types::{ChainSpec, Epoch, EthSpec, ForkName}; /// A type-alias to the tighten the definition of a production-intended `Client`. pub type ProductionClient = - Client, BeaconNodeBackend>>; + Client>; /// The beacon node `Client` that is used in production. /// diff --git a/beacon_node/store/src/database/interface.rs b/beacon_node/store/src/database/interface.rs index 5646f1179c..7e0a09a3e9 100644 --- a/beacon_node/store/src/database/interface.rs +++ b/beacon_node/store/src/database/interface.rs @@ -6,18 +6,17 @@ use crate::{ColumnIter, ColumnKeyIter, DBColumn, Error, ItemStore, Key, KeyValue use crate::{KeyValueStoreOp, StoreConfig, config::DatabaseBackend}; use std::collections::HashSet; use std::path::Path; -use types::EthSpec; -pub enum BeaconNodeBackend { +pub enum BeaconNodeBackend { #[cfg(feature = "leveldb")] - LevelDb(leveldb_impl::LevelDB), + LevelDb(leveldb_impl::LevelDB), #[cfg(feature = "redb")] - Redb(redb_impl::Redb), + Redb(redb_impl::Redb), } -impl ItemStore for BeaconNodeBackend {} +impl ItemStore for BeaconNodeBackend {} -impl KeyValueStore for BeaconNodeBackend { +impl KeyValueStore for BeaconNodeBackend { fn get_bytes(&self, column: DBColumn, key: &[u8]) -> Result>, Error> { match self { #[cfg(feature = "leveldb")] @@ -183,7 +182,7 @@ impl KeyValueStore for BeaconNodeBackend { } } -impl BeaconNodeBackend { +impl BeaconNodeBackend { pub fn open(config: &StoreConfig, path: &Path) -> Result { metrics::inc_counter_vec(&metrics::DISK_DB_TYPE, &[&config.backend.to_string()]); match config.backend { diff --git a/beacon_node/store/src/database/leveldb_impl.rs b/beacon_node/store/src/database/leveldb_impl.rs index 6e01648263..0531eb900e 100644 --- a/beacon_node/store/src/database/leveldb_impl.rs +++ b/beacon_node/store/src/database/leveldb_impl.rs @@ -15,15 +15,13 @@ use leveldb::{ options::{Options, ReadOptions}, }; use std::collections::HashSet; -use std::marker::PhantomData; use std::path::Path; -use types::{EthSpec, Hash256}; +use types::Hash256; use super::interface::WriteOptions; -pub struct LevelDB { +pub struct LevelDB { db: Database, - _phantom: PhantomData, } impl From for leveldb::options::WriteOptions { @@ -34,7 +32,7 @@ impl From for leveldb::options::WriteOptions { } } -impl LevelDB { +impl LevelDB { pub fn open(path: &Path) -> Result { let mut options = Options::new(); @@ -42,10 +40,7 @@ impl LevelDB { let db = Database::open(path, options)?; - Ok(Self { - db, - _phantom: PhantomData, - }) + Ok(Self { db }) } pub fn read_options(&self) -> ReadOptions<'_, BytesKey> { diff --git a/beacon_node/store/src/database/redb_impl.rs b/beacon_node/store/src/database/redb_impl.rs index 4077326eca..dc39f22114 100644 --- a/beacon_node/store/src/database/redb_impl.rs +++ b/beacon_node/store/src/database/redb_impl.rs @@ -3,17 +3,15 @@ use crate::{DBColumn, Error, KeyValueStoreOp}; use parking_lot::RwLock; use redb::TableDefinition; use std::collections::HashSet; -use std::{borrow::BorrowMut, marker::PhantomData, path::Path}; +use std::{borrow::BorrowMut, path::Path}; use strum::IntoEnumIterator; -use types::EthSpec; use super::interface::WriteOptions; pub const DB_FILE_NAME: &str = "database.redb"; -pub struct Redb { +pub struct Redb { db: RwLock, - _phantom: PhantomData, } impl From for redb::Durability { @@ -26,19 +24,16 @@ impl From for redb::Durability { } } -impl Redb { +impl Redb { pub fn open(path: &Path) -> Result { let db_file = path.join(DB_FILE_NAME); let db = redb::Database::create(db_file)?; for column in DBColumn::iter() { - Redb::::create_table(&db, column.into())?; + Self::create_table(&db, column.into())?; } - Ok(Self { - db: db.into(), - _phantom: PhantomData, - }) + Ok(Self { db: db.into() }) } fn create_table(db: &redb::Database, table_name: &str) -> Result<(), Error> { diff --git a/beacon_node/store/src/forwards_iter.rs b/beacon_node/store/src/forwards_iter.rs index 255b7d8eac..ef4312f506 100644 --- a/beacon_node/store/src/forwards_iter.rs +++ b/beacon_node/store/src/forwards_iter.rs @@ -9,7 +9,7 @@ pub type HybridForwardsBlockRootsIterator<'a, E, Hot, Cold> = pub type HybridForwardsStateRootsIterator<'a, E, Hot, Cold> = HybridForwardsIterator<'a, E, Hot, Cold>; -impl, Cold: ItemStore> HotColdDB { +impl HotColdDB { fn simple_forwards_iterator( &self, column: DBColumn, @@ -116,7 +116,7 @@ impl, Cold: ItemStore> HotColdDB } /// Forwards root iterator that makes use of a slot -> root mapping in the freezer DB. -pub struct FrozenForwardsIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> { +pub struct FrozenForwardsIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> { inner: ColumnIter<'a, Vec>, column: DBColumn, next_slot: Slot, @@ -124,9 +124,7 @@ pub struct FrozenForwardsIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemS _phantom: PhantomData<(E, Hot, Cold)>, } -impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> - FrozenForwardsIterator<'a, E, Hot, Cold> -{ +impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> FrozenForwardsIterator<'a, E, Hot, Cold> { /// `end_slot` is EXCLUSIVE here. pub fn new( store: &'a HotColdDB, @@ -148,7 +146,7 @@ impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> } } -impl, Cold: ItemStore> Iterator +impl Iterator for FrozenForwardsIterator<'_, E, Hot, Cold> { type Item = Result<(Hash256, Slot)>; @@ -199,7 +197,7 @@ impl Iterator for SimpleForwardsIterator { } /// Fusion of the above two approaches to forwards iteration. Fast and efficient. -pub enum HybridForwardsIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> { +pub enum HybridForwardsIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> { PreFinalization { iter: Box>, store: &'a HotColdDB, @@ -220,9 +218,7 @@ pub enum HybridForwardsIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemSto Finished, } -impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> - HybridForwardsIterator<'a, E, Hot, Cold> -{ +impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> HybridForwardsIterator<'a, E, Hot, Cold> { /// Construct a new hybrid iterator. /// /// The `get_state` closure should return a beacon state and final block/state root to backtrack @@ -349,7 +345,7 @@ impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> } } -impl, Cold: ItemStore> Iterator +impl Iterator for HybridForwardsIterator<'_, E, Hot, Cold> { type Item = Result<(Hash256, Slot)>; diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index e9b9de76e6..a625a97004 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -49,7 +49,7 @@ use zstd::{Decoder, Encoder}; /// Stores vector fields like the `block_roots` and `state_roots` separately, and only stores /// intermittent "restore point" states pre-finalization. #[derive(Debug)] -pub struct HotColdDB, Cold: ItemStore> { +pub struct HotColdDB { /// The slot and state root at the point where the database is split between hot and cold. /// /// States with slots less than `split.slot` are in the cold DB, while states with slots @@ -217,11 +217,11 @@ pub enum HotColdDBError { Rollback, } -impl HotColdDB, MemoryStore> { +impl HotColdDB { pub fn open_ephemeral( config: StoreConfig, spec: Arc, - ) -> Result, MemoryStore>, Error> { + ) -> Result, Error> { config.verify::()?; let hierarchy = config.hierarchy_config.to_moduli()?; @@ -258,7 +258,7 @@ impl HotColdDB, MemoryStore> { } } -impl HotColdDB, BeaconNodeBackend> { +impl HotColdDB { /// Open a new or existing database, with the given paths to the hot and cold DBs. /// /// The `migrate_schema` function is passed in so that the parent `BeaconChain` can provide @@ -451,7 +451,7 @@ impl HotColdDB, BeaconNodeBackend> { } } -impl, Cold: ItemStore> HotColdDB { +impl HotColdDB { fn cold_storage_strategy(&self, slot: Slot) -> Result { // The start slot for the freezer HDiff is always 0 Ok(self.hierarchy.storage_strategy(slot, Slot::new(0))?) @@ -3575,7 +3575,7 @@ impl, Cold: ItemStore> HotColdDB /// This function previously did a combination of freezer migration alongside pruning. Now it is /// *just* responsible for copying relevant data to the freezer, while pruning is implemented /// in `prune_hot_db`. -pub fn migrate_database, Cold: ItemStore>( +pub fn migrate_database( store: Arc>, finalized_state_root: Hash256, finalized_block_root: Hash256, @@ -3786,7 +3786,7 @@ pub enum StateSummaryIteratorError { /// Return the ancestor state root of a state beyond SlotsPerHistoricalRoot using the roots iterator /// and the store -pub fn get_ancestor_state_root<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore>( +pub fn get_ancestor_state_root<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore>( store: &'a HotColdDB, from_state: &'a BeaconState, target_slot: Slot, @@ -3993,7 +3993,7 @@ impl StoreItem for HotStateSummary { impl HotStateSummary { /// Construct a new summary of the given state. - pub fn new, Cold: ItemStore>( + pub fn new( store: &HotColdDB, state_root: Hash256, state: &BeaconState, diff --git a/beacon_node/store/src/invariants.rs b/beacon_node/store/src/invariants.rs index d251fb8800..4ec72b82bd 100644 --- a/beacon_node/store/src/invariants.rs +++ b/beacon_node/store/src/invariants.rs @@ -242,7 +242,7 @@ pub enum InvariantViolation { ColdStateBaseSummaryMissing { slot: Slot, base_slot: Slot }, } -impl, Cold: ItemStore> HotColdDB { +impl HotColdDB { /// Run all database invariant checks. /// /// The `ctx` parameter provides data from the beacon chain layer (fork choice, state cache, diff --git a/beacon_node/store/src/iter.rs b/beacon_node/store/src/iter.rs index 0cb803d1ed..cf1ab86ffe 100644 --- a/beacon_node/store/src/iter.rs +++ b/beacon_node/store/src/iter.rs @@ -13,12 +13,12 @@ use types::{ /// /// It is assumed that all ancestors for this object are stored in the database. If this is not the /// case, the iterator will start returning `None` prior to genesis. -pub trait AncestorIter<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore, I: Iterator> { +pub trait AncestorIter<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore, I: Iterator> { /// Returns an iterator over the roots of the ancestors of `self`. fn try_iter_ancestor_roots(&self, store: &'a HotColdDB) -> Option; } -impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> +impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> AncestorIter<'a, E, Hot, Cold, BlockRootsIterator<'a, E, Hot, Cold>> for SignedBeaconBlock { /// Iterates across all available prior block roots of `self`, starting at the most recent and ending @@ -37,7 +37,7 @@ impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> } } -impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> +impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> AncestorIter<'a, E, Hot, Cold, StateRootsIterator<'a, E, Hot, Cold>> for BeaconState { /// Iterates across all available prior state roots of `self`, starting at the most recent and ending @@ -51,13 +51,11 @@ impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> } } -pub struct StateRootsIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> { +pub struct StateRootsIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> { inner: RootsIterator<'a, E, Hot, Cold>, } -impl, Cold: ItemStore> Clone - for StateRootsIterator<'_, E, Hot, Cold> -{ +impl Clone for StateRootsIterator<'_, E, Hot, Cold> { fn clone(&self) -> Self { Self { inner: self.inner.clone(), @@ -65,7 +63,7 @@ impl, Cold: ItemStore> Clone } } -impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> StateRootsIterator<'a, E, Hot, Cold> { +impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> StateRootsIterator<'a, E, Hot, Cold> { pub fn new(store: &'a HotColdDB, beacon_state: &'a BeaconState) -> Self { Self { inner: RootsIterator::new(store, beacon_state), @@ -79,7 +77,7 @@ impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> StateRootsIterator<' } } -impl, Cold: ItemStore> Iterator +impl Iterator for StateRootsIterator<'_, E, Hot, Cold> { type Item = Result<(Hash256, Slot), Error>; @@ -99,13 +97,11 @@ impl, Cold: ItemStore> Iterator /// exhausted. /// /// Returns `None` for roots prior to genesis or when there is an error reading from `Store`. -pub struct BlockRootsIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> { +pub struct BlockRootsIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> { inner: RootsIterator<'a, E, Hot, Cold>, } -impl, Cold: ItemStore> Clone - for BlockRootsIterator<'_, E, Hot, Cold> -{ +impl Clone for BlockRootsIterator<'_, E, Hot, Cold> { fn clone(&self) -> Self { Self { inner: self.inner.clone(), @@ -113,7 +109,7 @@ impl, Cold: ItemStore> Clone } } -impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> BlockRootsIterator<'a, E, Hot, Cold> { +impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> BlockRootsIterator<'a, E, Hot, Cold> { /// Create a new iterator over all block roots in the given `beacon_state` and prior states. pub fn new(store: &'a HotColdDB, beacon_state: &'a BeaconState) -> Self { Self { @@ -138,7 +134,7 @@ impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> BlockRootsIterator<' } } -impl, Cold: ItemStore> Iterator +impl Iterator for BlockRootsIterator<'_, E, Hot, Cold> { type Item = Result<(Hash256, Slot), Error>; @@ -151,13 +147,13 @@ impl, Cold: ItemStore> Iterator } /// Iterator over state and block roots that backtracks using the vectors from a `BeaconState`. -pub struct RootsIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> { +pub struct RootsIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> { store: &'a HotColdDB, beacon_state: Cow<'a, BeaconState>, slot: Slot, } -impl, Cold: ItemStore> Clone for RootsIterator<'_, E, Hot, Cold> { +impl Clone for RootsIterator<'_, E, Hot, Cold> { fn clone(&self) -> Self { Self { store: self.store, @@ -167,7 +163,7 @@ impl, Cold: ItemStore> Clone for RootsIterator< } } -impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> RootsIterator<'a, E, Hot, Cold> { +impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> RootsIterator<'a, E, Hot, Cold> { pub fn new(store: &'a HotColdDB, beacon_state: &'a BeaconState) -> Self { Self { store, @@ -234,9 +230,7 @@ impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> RootsIterator<'a, E, } } -impl, Cold: ItemStore> Iterator - for RootsIterator<'_, E, Hot, Cold> -{ +impl Iterator for RootsIterator<'_, E, Hot, Cold> { /// (block_root, state_root, slot) type Item = Result<(Hash256, Hash256, Slot), Error>; @@ -246,15 +240,13 @@ impl, Cold: ItemStore> Iterator } /// Block iterator that uses the `parent_root` of each block to backtrack. -pub struct ParentRootBlockIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> { +pub struct ParentRootBlockIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> { store: &'a HotColdDB, next_block_root: Hash256, _phantom: PhantomData, } -impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> - ParentRootBlockIterator<'a, E, Hot, Cold> -{ +impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> ParentRootBlockIterator<'a, E, Hot, Cold> { pub fn new(store: &'a HotColdDB, start_block_root: Hash256) -> Self { Self { store, @@ -283,7 +275,7 @@ impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> } } -impl, Cold: ItemStore> Iterator +impl Iterator for ParentRootBlockIterator<'_, E, Hot, Cold> { type Item = Result<(Hash256, SignedBeaconBlock>), Error>; @@ -295,11 +287,11 @@ impl, Cold: ItemStore> Iterator #[derive(Clone)] /// Extends `BlockRootsIterator`, returning `SignedBeaconBlock` instances, instead of their roots. -pub struct BlockIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> { +pub struct BlockIterator<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> { roots: BlockRootsIterator<'a, E, Hot, Cold>, } -impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> BlockIterator<'a, E, Hot, Cold> { +impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> BlockIterator<'a, E, Hot, Cold> { /// Create a new iterator over all blocks in the given `beacon_state` and prior states. pub fn new(store: &'a HotColdDB, beacon_state: &'a BeaconState) -> Self { Self { @@ -324,9 +316,7 @@ impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> BlockIterator<'a, E, } } -impl, Cold: ItemStore> Iterator - for BlockIterator<'_, E, Hot, Cold> -{ +impl Iterator for BlockIterator<'_, E, Hot, Cold> { type Item = Result>, Error>; fn next(&mut self) -> Option { @@ -338,7 +328,7 @@ impl, Cold: ItemStore> Iterator /// /// Return `Err(HistoryUnavailable)` in the case where no more backtrack states are available /// due to weak subjectivity sync. -fn next_historical_root_backtrack_state, Cold: ItemStore>( +fn next_historical_root_backtrack_state( store: &HotColdDB, current_state: &BeaconState, ) -> Result, Error> { @@ -386,7 +376,7 @@ mod test { harness.get_current_state() } - fn get_store() -> HotColdDB, MemoryStore> { + fn get_store() -> HotColdDB { let store = HotColdDB::open_ephemeral(Config::default(), Arc::new(E::default_spec())).unwrap(); // Init achor info so anchor slot is set. Use a random block as it is only used for the diff --git a/beacon_node/store/src/lib.rs b/beacon_node/store/src/lib.rs index bd8caa3ad5..56cdd18fbe 100644 --- a/beacon_node/store/src/lib.rs +++ b/beacon_node/store/src/lib.rs @@ -46,7 +46,7 @@ pub type ColumnKeyIter<'a, K> = Box> + 'a>; pub type RawEntryIter<'a> = Result, Vec), Error>> + 'a>, Error>; -pub trait KeyValueStore: Sync + Send + Sized + 'static { +pub trait KeyValueStore: Sync + Send + Sized + 'static { /// Retrieve some bytes in `column` with `key`. fn get_bytes(&self, column: DBColumn, key: &[u8]) -> Result>, Error>; @@ -177,7 +177,7 @@ pub enum KeyValueStoreOp { DeleteKey(DBColumn, Vec), } -pub trait ItemStore: KeyValueStore + Sync + Send + Sized + 'static { +pub trait ItemStore: KeyValueStore + Sync + Send + Sized + 'static { /// Store an item in `Self`. fn put(&self, key: &Hash256, item: &I) -> Result<(), Error> { let column = I::db_column(); @@ -493,7 +493,7 @@ mod tests { } } - fn test_impl(store: impl ItemStore) { + fn test_impl(store: impl ItemStore) { let key = Hash256::random(); let item = StorableThing { a: 1, b: 42 }; @@ -531,7 +531,7 @@ mod tests { #[test] fn exists() { - let store = MemoryStore::::open(); + let store = MemoryStore::open(); let key = Hash256::random(); let item = StorableThing { a: 1, b: 42 }; diff --git a/beacon_node/store/src/memory_store.rs b/beacon_node/store/src/memory_store.rs index 6baef61c9d..8be9278d90 100644 --- a/beacon_node/store/src/memory_store.rs +++ b/beacon_node/store/src/memory_store.rs @@ -4,28 +4,24 @@ use crate::{ }; use parking_lot::RwLock; use std::collections::{BTreeMap, HashSet}; -use std::marker::PhantomData; -use types::*; type DBMap = BTreeMap>; /// A thread-safe `BTreeMap` wrapper. -pub struct MemoryStore { +pub struct MemoryStore { db: RwLock, - _phantom: PhantomData, } -impl MemoryStore { +impl MemoryStore { /// Create a new, empty database. pub fn open() -> Self { Self { db: RwLock::new(BTreeMap::new()), - _phantom: PhantomData, } } } -impl KeyValueStore for MemoryStore { +impl KeyValueStore for MemoryStore { /// Get the value of some key from the database. Returns `None` if the key does not exist. fn get_bytes(&self, col: DBColumn, key: &[u8]) -> Result>, Error> { let column_key = BytesKey::from_vec(get_key_for_col(col, key)); @@ -148,4 +144,4 @@ impl KeyValueStore for MemoryStore { } } -impl ItemStore for MemoryStore {} +impl ItemStore for MemoryStore {} diff --git a/beacon_node/store/src/reconstruct.rs b/beacon_node/store/src/reconstruct.rs index 04a519af02..2fb40daa0d 100644 --- a/beacon_node/store/src/reconstruct.rs +++ b/beacon_node/store/src/reconstruct.rs @@ -15,8 +15,8 @@ use types::{EthSpec, Slot}; impl HotColdDB where E: EthSpec, - Hot: ItemStore, - Cold: ItemStore, + Hot: ItemStore, + Cold: ItemStore, { pub fn reconstruct_historic_states( self: &Arc, diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index 353893026b..848834b4d8 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -79,7 +79,7 @@ impl ForkChoiceTest { /// Get a value from the `ForkChoice` instantiation. fn get(&self, func: T) -> U where - T: Fn(&BeaconForkChoiceStore, MemoryStore>) -> U, + T: Fn(&BeaconForkChoiceStore) -> U, { func( self.harness diff --git a/database_manager/src/lib.rs b/database_manager/src/lib.rs index 608400fa7e..2509b500e0 100644 --- a/database_manager/src/lib.rs +++ b/database_manager/src/lib.rs @@ -55,7 +55,7 @@ pub fn display_db_version( let blobs_path = client_config.get_blobs_db_path(); let mut version = CURRENT_SCHEMA_VERSION; - HotColdDB::, BeaconNodeBackend>::open( + HotColdDB::::open( &hot_path, &cold_path, &blobs_path, @@ -143,13 +143,13 @@ pub fn inspect_db( let mut num_keys = 0; let sub_db = if inspect_config.freezer { - BeaconNodeBackend::::open(&client_config.store, &cold_path) + BeaconNodeBackend::open(&client_config.store, &cold_path) .map_err(|e| format!("Unable to open freezer DB: {e:?}"))? } else if inspect_config.blobs_db { - BeaconNodeBackend::::open(&client_config.store, &blobs_path) + BeaconNodeBackend::open(&client_config.store, &blobs_path) .map_err(|e| format!("Unable to open blobs DB: {e:?}"))? } else { - BeaconNodeBackend::::open(&client_config.store, &hot_path) + BeaconNodeBackend::open(&client_config.store, &hot_path) .map_err(|e| format!("Unable to open hot DB: {e:?}"))? }; @@ -264,17 +264,17 @@ pub fn compact_db( let (sub_db, db_name) = if compact_config.freezer { ( - BeaconNodeBackend::::open(&client_config.store, &cold_path)?, + BeaconNodeBackend::open(&client_config.store, &cold_path)?, "freezer_db", ) } else if compact_config.blobs_db { ( - BeaconNodeBackend::::open(&client_config.store, &blobs_path)?, + BeaconNodeBackend::open(&client_config.store, &blobs_path)?, "blobs_db", ) } else { ( - BeaconNodeBackend::::open(&client_config.store, &hot_path)?, + BeaconNodeBackend::open(&client_config.store, &hot_path)?, "hot_db", ) }; @@ -309,7 +309,7 @@ pub fn migrate_db( let mut from = CURRENT_SCHEMA_VERSION; let to = migrate_config.to; - let db = HotColdDB::, BeaconNodeBackend>::open( + let db = HotColdDB::::open( &hot_path, &cold_path, &blobs_path, @@ -339,7 +339,7 @@ pub fn prune_payloads( let cold_path = client_config.get_freezer_db_path(); let blobs_path = client_config.get_blobs_db_path(); - let db = HotColdDB::, BeaconNodeBackend>::open( + let db = HotColdDB::::open( &hot_path, &cold_path, &blobs_path, @@ -363,7 +363,7 @@ pub fn prune_blobs( let cold_path = client_config.get_freezer_db_path(); let blobs_path = client_config.get_blobs_db_path(); - let db = HotColdDB::, BeaconNodeBackend>::open( + let db = HotColdDB::::open( &hot_path, &cold_path, &blobs_path, @@ -398,7 +398,7 @@ pub fn prune_states( let cold_path = client_config.get_freezer_db_path(); let blobs_path = client_config.get_blobs_db_path(); - let db = HotColdDB::, BeaconNodeBackend>::open( + let db = HotColdDB::::open( &hot_path, &cold_path, &blobs_path, From b5d5644eebcb889b025c13b084c30ac1025adb59 Mon Sep 17 00:00:00 2001 From: Daniel Knopik <107140945+dknopik@users.noreply.github.com> Date: Thu, 21 May 2026 22:00:16 +0200 Subject: [PATCH 6/7] Add getBlobsV3 to `LIGHTHOUSE_CAPABILITIES` (#9330) Forgot to add `ENGINE_GET_BLOBS_V3` to `LIGHTHOUSE_CAPABILITIES`. Add `ENGINE_GET_BLOBS_V3` to `LIGHTHOUSE_CAPABILITIES`. Co-Authored-By: Daniel Knopik --- beacon_node/execution_layer/src/engine_api/http.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index 110e155c77..7c63f78a22 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -94,6 +94,7 @@ pub static LIGHTHOUSE_CAPABILITIES: &[&str] = &[ ENGINE_GET_CLIENT_VERSION_V1, ENGINE_GET_BLOBS_V1, ENGINE_GET_BLOBS_V2, + ENGINE_GET_BLOBS_V3, ]; /// We opt to initialize the JsonClientVersionV1 rather than the ClientVersionV1 From 60abd4b5b985f5ef47baa799c43c085521e3e596 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Thu, 21 May 2026 23:21:20 -0700 Subject: [PATCH 7/7] Gloas alpha spec 8 (#9315) https://github.com/ethereum/consensus-specs/releases/tag/v1.7.0-alpha.8 Co-Authored-By: Eitan Seri-Levi Co-Authored-By: Michael Sproul --- beacon_node/beacon_chain/src/beacon_chain.rs | 30 ++- .../src/block_production/gloas.rs | 64 +++-- .../beacon_chain/src/execution_payload.rs | 20 +- .../gossip_verified_bid.rs | 98 +++++++- .../src/payload_bid_verification/mod.rs | 2 +- .../src/payload_bid_verification/tests.rs | 15 +- .../gossip_verified_proposer_preferences.rs | 47 +++- .../proposer_preferences_verification/mod.rs | 2 +- .../proposer_preference_cache.rs | 2 +- .../tests.rs | 6 +- .../tests/payload_invalidation.rs | 1 + beacon_node/execution_layer/src/engine_api.rs | 43 ++-- .../src/engine_api/json_structures.rs | 5 + beacon_node/execution_layer/src/lib.rs | 9 +- .../src/test_utils/mock_builder.rs | 7 +- .../src/test_utils/mock_execution_layer.rs | 7 +- beacon_node/http_api/tests/tests.rs | 2 +- .../gossip_methods.rs | 2 +- .../mainnet/config.yaml | 4 +- consensus/fork_choice/src/fork_choice.rs | 96 +++++--- consensus/proto_array/src/error.rs | 5 + .../src/fork_choice_test_definition.rs | 6 +- consensus/proto_array/src/proto_array.rs | 99 ++++++-- .../src/proto_array_fork_choice.rs | 30 +++ .../process_operations.rs | 44 ++-- .../state_processing/src/upgrade/gloas.rs | 94 ++++--- consensus/types/configs/mainnet.yaml | 4 +- .../types/src/builder/proposer_preferences.rs | 2 +- consensus/types/src/core/chain_spec.rs | 92 ++++++- consensus/types/src/state/beacon_state.rs | 4 +- testing/ef_tests/Makefile | 2 +- testing/ef_tests/src/cases/fork_choice.rs | 233 +++++++++++++++++- testing/ef_tests/src/handler.rs | 13 +- testing/ef_tests/tests/tests.rs | 6 + .../src/test_rig.rs | 8 +- .../src/proposer_preferences_service.rs | 2 +- 36 files changed, 863 insertions(+), 243 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 2259e1d809..db8f55a18a 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -96,8 +96,8 @@ use eth2::types::{ SseExtendedPayloadAttributes, SseHead, }; use execution_layer::{ - BlockProposalContents, BlockProposalContentsType, BuilderParams, ChainHealth, ExecutionLayer, - FailedCondition, PayloadAttributes, PayloadStatus, + BlockProposalContents, BlockProposalContentsType, BuilderParams, ChainHealth, + DEFAULT_GAS_LIMIT, ExecutionLayer, FailedCondition, PayloadAttributes, PayloadStatus, }; use fixed_bytes::FixedBytesExtended; use fork_choice::{ @@ -2185,12 +2185,20 @@ impl BeaconChain { // TODO(gloas) do we want to use a dedicated envelope cache instead? // Maybe the new gloas DA cache? (Or should the gloas DA cache use - // the envelopes_times_cache internally?) + // the envelopes_times_cache internally? + // The payload is considered present only if it was observed before + // the payload due deadline (PAYLOAD_DUE_BPS into the slot). + let payload_due = self.spec.get_payload_due(); let payload_present = self .envelope_times_cache .read() .cache - .contains_key(&beacon_block_root); + .get(&beacon_block_root) + .and_then(|entry| entry.timestamps.observed) + .is_some_and(|observed| { + let slot_start = self.slot_clock.start_of(request_slot); + slot_start.is_some_and(|start| observed.saturating_sub(start) < payload_due) + }); // TODO(EIP-7732): Check blob data availability. For now, default to true. let blob_data_available = true; @@ -6476,6 +6484,19 @@ impl BeaconChain { None }; + let target_gas_limit = if prepare_slot_fork.gloas_enabled() { + let proposer_gas_limit = execution_layer.get_proposer_gas_limit(proposer).await; + if proposer_gas_limit.is_none() { + warn!( + %proposer, + "No proposer gas limit configured, falling back to parent gas limit" + ); + } + proposer_gas_limit.or(Some(DEFAULT_GAS_LIMIT)) + } else { + None + }; + let payload_attributes = PayloadAttributes::new( self.slot_clock .start_of(prepare_slot) @@ -6486,6 +6507,7 @@ impl BeaconChain { withdrawals.map(Into::into), parent_beacon_block_root, slot_number, + target_gas_limit, ); execution_layer diff --git a/beacon_node/beacon_chain/src/block_production/gloas.rs b/beacon_node/beacon_chain/src/block_production/gloas.rs index 6510c20ba7..82dad6f6ad 100644 --- a/beacon_node/beacon_chain/src/block_production/gloas.rs +++ b/beacon_node/beacon_chain/src/block_production/gloas.rs @@ -2,11 +2,13 @@ use std::collections::{HashMap, HashSet}; use std::marker::PhantomData; use std::sync::Arc; +use proto_array::PayloadStatus; + use bls::{PublicKeyBytes, Signature}; use execution_layer::{ - BlockProposalContentsGloas, BuilderParams, PayloadAttributes, PayloadParameters, + BlockProposalContentsGloas, BuilderParams, DEFAULT_GAS_LIMIT, PayloadAttributes, + PayloadParameters, }; -use fork_choice::PayloadStatus; use operation_pool::CompactAttestationRef; use ssz::Encode; use state_processing::common::{get_attesting_indices_from_state, get_indexed_payload_attestation}; @@ -150,8 +152,24 @@ impl BeaconChain { verification: ProduceBlockVerification, builder_boost_factor: Option, ) -> Result, BlockProductionError> { - // Extract the parent's execution requests from the envelope (if parent was full). - let parent_execution_requests = if parent_payload_status == PayloadStatus::Full { + let parent_root = if state.slot() > 0 { + *state + .get_block_root(state.slot() - 1) + .map_err(|_| BlockProductionError::UnableToGetBlockRootFromState)? + } else { + state.latest_block_header().canonical_root() + }; + + let should_build_on_full = self + .canonical_head + .fork_choice_read_lock() + .should_build_on_full(&parent_root, parent_payload_status) + .map_err(|e| { + BlockProductionError::BeaconChain(Box::new(BeaconChainError::ForkChoiceError(e))) + })?; + + // Extract the parent's execution requests from the envelope (if building on full). + let parent_execution_requests = if should_build_on_full { parent_envelope .as_ref() .map(|env| env.message.execution_requests.clone()) @@ -197,7 +215,7 @@ impl BeaconChain { .clone() .produce_execution_payload_bid( state, - parent_payload_status, + should_build_on_full, parent_envelope, produce_at_slot, BID_VALUE_SELF_BUILD, @@ -700,12 +718,12 @@ impl BeaconChain { /// data needed to construct the `ExecutionPayloadEnvelope` after the beacon block is /// created, plus the EL block value and `should_override_builder` flag used by the /// caller to compare against any cached p2p builder bid. - #[allow(clippy::type_complexity)] + #[allow(clippy::type_complexity, clippy::too_many_arguments)] #[instrument(level = "debug", skip_all)] pub async fn produce_execution_payload_bid( self: Arc, state: BeaconState, - parent_payload_status: PayloadStatus, + should_build_on_full: bool, parent_envelope: Option>>, produce_at_slot: Slot, bid_value: u64, @@ -751,20 +769,18 @@ impl BeaconChain { let parent_bid = state.latest_execution_payload_bid()?; - // TODO(gloas): need should_extend_payload check here as well let parent_block_slot = state.latest_block_header().slot; let parent_is_pre_gloas = !self .spec .fork_name_at_slot::(parent_block_slot) .gloas_enabled(); - let parent_block_hash = - if parent_payload_status == PayloadStatus::Full || parent_is_pre_gloas { - // Build on parent bid's payload. - parent_bid.block_hash - } else { - // Skip parent bid's payload. For genesis this is the EL genesis hash. - parent_bid.parent_block_hash - }; + let parent_block_hash = if should_build_on_full || parent_is_pre_gloas { + // Build on parent bid's payload. + parent_bid.block_hash + } else { + // Skip parent bid's payload. For genesis this is the EL genesis hash. + parent_bid.parent_block_hash + }; // TODO(gloas) this should be BlockProductionVersion::V4 // V3 is okay for now as long as we're not connected to a builder @@ -953,10 +969,7 @@ fn get_execution_payload_gloas( compute_timestamp_at_slot(state, state.slot(), spec).map_err(BeaconStateError::from)?; let random = *state.get_randao_mix(current_epoch)?; - // TODO(gloas): this gas limit calc is not necessarily right let parent_bid = state.latest_execution_payload_bid()?; - let latest_gas_limit = parent_bid.gas_limit; - let is_parent_block_full = parent_block_hash == parent_bid.block_hash; let withdrawals = if is_parent_block_full { @@ -992,7 +1005,6 @@ fn get_execution_payload_gloas( random, proposer_index, parent_block_hash, - latest_gas_limit, builder_params, withdrawals, parent_beacon_block_root, @@ -1020,7 +1032,6 @@ async fn prepare_execution_payload( random: Hash256, proposer_index: u64, parent_block_hash: ExecutionBlockHash, - parent_gas_limit: u64, builder_params: BuilderParams, withdrawals: Vec, parent_beacon_block_root: Hash256, @@ -1058,6 +1069,10 @@ where .get_suggested_fee_recipient(proposer_index) .await; let slot_number = Some(builder_params.slot.as_u64()); + let target_gas_limit = execution_layer + .get_proposer_gas_limit(proposer_index) + .await + .unwrap_or(DEFAULT_GAS_LIMIT); let payload_attributes = PayloadAttributes::new( timestamp, @@ -1066,13 +1081,12 @@ where Some(withdrawals), Some(parent_beacon_block_root), slot_number, + Some(target_gas_limit), ); - - let target_gas_limit = execution_layer.get_proposer_gas_limit(proposer_index).await; let payload_parameters = PayloadParameters { parent_hash: parent_block_hash, - parent_gas_limit, - proposer_gas_limit: target_gas_limit, + parent_gas_limit: None, + proposer_gas_limit: Some(target_gas_limit), payload_attributes: &payload_attributes, forkchoice_update_params: &forkchoice_update_params, current_fork: fork, diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index 16542eea2d..c8976fc6a8 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -342,7 +342,7 @@ pub fn get_execution_payload( Ok(join_handle) } -/// Prepares an execution payload for inclusion in a block. +/// Prepares an execution payload (pre-gloas) for inclusion in a block. /// /// ## Errors /// @@ -373,6 +373,13 @@ where { let spec = &chain.spec; let fork = spec.fork_name_at_slot::(builder_params.slot); + + if fork.gloas_enabled() { + return Err(BlockProductionError::InvalidBlockVariant( + "Called pre-gloas prepare_execution_payload on a gloas block".to_string(), + )); + } + let execution_layer = chain .execution_layer .as_ref() @@ -403,25 +410,20 @@ where .get_suggested_fee_recipient(proposer_index) .await; - let slot_number = if fork.gloas_enabled() { - Some(builder_params.slot.as_u64()) - } else { - None - }; - let payload_attributes = PayloadAttributes::new( timestamp, random, suggested_fee_recipient, withdrawals, parent_beacon_block_root, - slot_number, + None, + None, ); let target_gas_limit = execution_layer.get_proposer_gas_limit(proposer_index).await; let payload_parameters = PayloadParameters { parent_hash, - parent_gas_limit: latest_execution_payload_header_gas_limit, + parent_gas_limit: Some(latest_execution_payload_header_gas_limit), proposer_gas_limit: target_gas_limit, payload_attributes: &payload_attributes, forkchoice_update_params: &forkchoice_update_params, diff --git a/beacon_node/beacon_chain/src/payload_bid_verification/gossip_verified_bid.rs b/beacon_node/beacon_chain/src/payload_bid_verification/gossip_verified_bid.rs index 1f3f074598..354705b92c 100644 --- a/beacon_node/beacon_chain/src/payload_bid_verification/gossip_verified_bid.rs +++ b/beacon_node/beacon_chain/src/payload_bid_verification/gossip_verified_bid.rs @@ -43,9 +43,6 @@ pub(crate) fn verify_bid_consistency( if bid.fee_recipient != proposer_preferences.message.fee_recipient { return Err(PayloadBidError::InvalidFeeRecipient); } - if bid.gas_limit != proposer_preferences.message.gas_limit { - return Err(PayloadBidError::InvalidGasLimit); - } let max_blobs_per_block = spec.max_blobs_per_block(bid_slot.epoch(E::slots_per_epoch())) as usize; @@ -161,7 +158,23 @@ impl GossipVerifiedPayloadBid { }); } - // TODO(gloas) [IGNORE] bid.parent_block_hash is the block hash of a known execution payload in fork choice. + // TODO(gloas): [IGNORE] bid.parent_block_hash is the block hash of a known execution + // payload in fork choice. + + // TODO(gloas): This uses head state's bid gas_limit as parent_gas_limit, which is only + // correct when the bid's parent is the head. If the parent is an ancestor further back + // this check may be inaccurate. Fixing this requires storing + // gas_limit in fork choice or looking it up from the store by parent_block_hash. Taking the above + // TODO into consideration maybe should persist parent block hash and gas limit in fork choice? + if let Ok(parent_bid) = head_state.latest_execution_payload_bid() + && !is_gas_limit_target_compatible( + parent_bid.gas_limit, + signed_bid.message.gas_limit, + proposer_preferences.message.target_gas_limit, + )? + { + return Err(PayloadBidError::InvalidGasLimit); + } drop(fork_choice); @@ -263,8 +276,36 @@ impl BeaconChain { } } +/// Check if `gas_limit` is compatible with `target_gas_limit` under the +/// EIP-1559 transition rule from `parent_gas_limit`. +pub fn is_gas_limit_target_compatible( + parent_gas_limit: u64, + gas_limit: u64, + target_gas_limit: u64, +) -> Result { + let max_gas_limit_difference = (parent_gas_limit / 1024) + .max(1) + .checked_sub(1) + .ok_or(PayloadBidError::InvalidGasLimit)?; + let min_gas_limit = parent_gas_limit + .checked_sub(max_gas_limit_difference) + .ok_or(PayloadBidError::InvalidGasLimit)?; + let max_gas_limit = parent_gas_limit + .checked_add(max_gas_limit_difference) + .ok_or(PayloadBidError::InvalidGasLimit)?; + + if target_gas_limit >= min_gas_limit && target_gas_limit <= max_gas_limit { + Ok(gas_limit == target_gas_limit) + } else if target_gas_limit > max_gas_limit { + Ok(gas_limit == max_gas_limit) + } else { + Ok(gas_limit == min_gas_limit) + } +} + #[cfg(test)] mod tests { + use super::is_gas_limit_target_compatible; use bls::Signature; use kzg::KzgCommitment; use ssz_types::VariableList; @@ -288,11 +329,14 @@ mod tests { } } - fn make_preferences(fee_recipient: Address, gas_limit: u64) -> SignedProposerPreferences { + fn make_preferences( + fee_recipient: Address, + target_gas_limit: u64, + ) -> SignedProposerPreferences { SignedProposerPreferences { message: ProposerPreferences { fee_recipient, - gas_limit, + target_gas_limit, ..ProposerPreferences::default() }, signature: Signature::empty(), @@ -382,13 +426,41 @@ mod tests { } #[test] - fn test_gas_limit_mismatch() { - let (state, spec) = state_and_spec(); - let current_slot = Slot::new(10); - let bid = make_bid(current_slot, Address::ZERO, 30_000_000); - let prefs = make_preferences(Address::ZERO, 50_000_000); + fn test_is_gas_limit_target_compatible_increase_within_limit() { + assert!(is_gas_limit_target_compatible(60_000_000, 60_000_100, 60_000_100).unwrap()); + } - let result = verify_bid_consistency::(&bid, current_slot, &prefs, &state, &spec); - assert!(matches!(result, Err(PayloadBidError::InvalidGasLimit))); + #[test] + fn test_is_gas_limit_target_compatible_increase_exceeding_limit() { + // max_diff = 60_000_000 / 1024 - 1 = 58_592 + // max_gas_limit = 60_000_000 + 58_592 = 60_058_592 + assert!(is_gas_limit_target_compatible(60_000_000, 60_058_592, 100_000_000).unwrap()); + } + + #[test] + fn test_is_gas_limit_target_compatible_increase_exceeding_off_by_one() { + assert!(!is_gas_limit_target_compatible(60_000_000, 60_058_593, 100_000_000).unwrap()); + } + + #[test] + fn test_is_gas_limit_target_compatible_decrease_within_limit() { + assert!(is_gas_limit_target_compatible(60_000_000, 59_999_990, 59_999_990).unwrap()); + } + + #[test] + fn test_is_gas_limit_target_compatible_decrease_exceeding_limit() { + // min_gas_limit = 60_000_000 - 58_592 = 59_941_408 + assert!(is_gas_limit_target_compatible(60_000_000, 59_941_408, 30_000_000).unwrap()); + } + + #[test] + fn test_is_gas_limit_target_compatible_target_equals_parent() { + assert!(is_gas_limit_target_compatible(60_000_000, 60_000_000, 60_000_000).unwrap()); + } + + #[test] + fn test_is_gas_limit_target_compatible_parent_underflows() { + // parent=1023: max(1023/1024, 1) - 1 = max(0, 1) - 1 = 0, no change allowed + assert!(is_gas_limit_target_compatible(1023, 1023, 60_000_000).unwrap()); } } diff --git a/beacon_node/beacon_chain/src/payload_bid_verification/mod.rs b/beacon_node/beacon_chain/src/payload_bid_verification/mod.rs index 514695f5c0..a40fd14872 100644 --- a/beacon_node/beacon_chain/src/payload_bid_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_bid_verification/mod.rs @@ -48,7 +48,7 @@ pub enum PayloadBidError { }, /// The bids fee recipient doesn't match the proposer preferences fee recipient. InvalidFeeRecipient, - /// The bids gas limit doesn't match the proposer preferences gas limit. + /// The bid's gas limit is not compatible with the proposer's target gas limit. InvalidGasLimit, /// The bids execution payment is non-zero ExecutionPaymentNonZero { execution_payment: u64 }, diff --git a/beacon_node/beacon_chain/src/payload_bid_verification/tests.rs b/beacon_node/beacon_chain/src/payload_bid_verification/tests.rs index c68e6d9d32..ccdf64d41d 100644 --- a/beacon_node/beacon_chain/src/payload_bid_verification/tests.rs +++ b/beacon_node/beacon_chain/src/payload_bid_verification/tests.rs @@ -101,6 +101,17 @@ impl TestContext { root: Hash256::ZERO, }; + // Set a non-zero gas_limit on latest_execution_payload_bid so the gas limit + // compatibility check doesn't reject all bids at genesis. + if let Ok(bid) = state.latest_execution_payload_bid_mut() { + bid.gas_limit = 30_000_000; + } + // Update body_root to reflect the modified bid (genesis block embeds it). + let genesis_body_root = genesis_block(&state, &spec) + .expect("should build genesis block") + .body_root(); + state.latest_block_header_mut().body_root = genesis_body_root; + let inactive_keypair = &keypairs[NUM_BUILDERS]; let inactive_creds = builder_withdrawal_credentials(&inactive_keypair.pk, &spec); let inactive_builder_index = state @@ -248,7 +259,7 @@ fn make_signed_preferences( proposal_slot: Slot, validator_index: u64, fee_recipient: Address, - gas_limit: u64, + target_gas_limit: u64, ) -> Arc { Arc::new(SignedProposerPreferences { message: ProposerPreferences { @@ -256,7 +267,7 @@ fn make_signed_preferences( proposal_slot, validator_index, fee_recipient, - gas_limit, + target_gas_limit, }, signature: Signature::empty(), }) diff --git a/beacon_node/beacon_chain/src/proposer_preferences_verification/gossip_verified_proposer_preferences.rs b/beacon_node/beacon_chain/src/proposer_preferences_verification/gossip_verified_proposer_preferences.rs index 4ba33fde72..586721d8c1 100644 --- a/beacon_node/beacon_chain/src/proposer_preferences_verification/gossip_verified_proposer_preferences.rs +++ b/beacon_node/beacon_chain/src/proposer_preferences_verification/gossip_verified_proposer_preferences.rs @@ -18,13 +18,16 @@ pub(crate) fn verify_preferences_consistency( preferences: &ProposerPreferences, current_slot: Slot, head_state: &BeaconState, + spec: &ChainSpec, ) -> Result<(), ProposerPreferencesError> { let proposal_slot = preferences.proposal_slot; let validator_index = preferences.validator_index; let current_epoch = current_slot.epoch(E::slots_per_epoch()); let proposal_epoch = proposal_slot.epoch(E::slots_per_epoch()); - if proposal_epoch < current_epoch || proposal_epoch > current_epoch.saturating_add(1u64) { + if proposal_epoch < current_epoch + || proposal_epoch > current_epoch.saturating_add(spec.min_seed_lookahead) + { return Err(ProposerPreferencesError::InvalidProposalEpoch { proposal_epoch }); } @@ -35,7 +38,7 @@ pub(crate) fn verify_preferences_consistency( }); } - if !head_state.is_valid_proposal_slot(preferences)? { + if !head_state.is_valid_proposal_slot(preferences, spec)? { return Err(ProposerPreferencesError::InvalidProposalSlot { validator_index, proposal_slot, @@ -83,7 +86,12 @@ impl GossipVerifiedProposerPreferences { }); } - verify_preferences_consistency(&signed_preferences.message, current_slot, head_state)?; + verify_preferences_consistency( + &signed_preferences.message, + current_slot, + head_state, + ctx.spec, + )?; // Verify signature proposer_preferences_signature_set( @@ -155,11 +163,13 @@ impl BeaconChain { #[cfg(test)] mod tests { use types::{ - Address, BeaconState, EthSpec, Hash256, MinimalEthSpec, ProposerPreferences, Slot, + Address, BeaconState, ChainSpec, EthSpec, Hash256, MinimalEthSpec, ProposerPreferences, + Slot, }; use super::verify_preferences_consistency; use crate::proposer_preferences_verification::ProposerPreferencesError; + use crate::test_utils::{fork_name_from_env, test_spec}; type E = MinimalEthSpec; @@ -169,20 +179,28 @@ mod tests { proposal_slot, validator_index, fee_recipient: Address::ZERO, - gas_limit: 30_000_000, + target_gas_limit: 30_000_000, } } fn state() -> BeaconState { - BeaconState::new(0, <_>::default(), &E::default_spec()) + let spec = spec(); + BeaconState::new(0, <_>::default(), &spec) + } + + fn spec() -> ChainSpec { + test_spec::() } #[test] fn test_invalid_epoch_too_old() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } let current_slot = Slot::new(2 * E::slots_per_epoch()); let prefs = make_preferences(Slot::new(3), 0); - let result = verify_preferences_consistency::(&prefs, current_slot, &state()); + let result = verify_preferences_consistency::(&prefs, current_slot, &state(), &spec()); assert!(matches!( result, Err(ProposerPreferencesError::InvalidProposalEpoch { .. }) @@ -191,10 +209,13 @@ mod tests { #[test] fn test_invalid_epoch_too_far_ahead() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } let current_slot = Slot::new(E::slots_per_epoch()); let prefs = make_preferences(Slot::new(3 * E::slots_per_epoch() + 1), 0); - let result = verify_preferences_consistency::(&prefs, current_slot, &state()); + let result = verify_preferences_consistency::(&prefs, current_slot, &state(), &spec()); assert!(matches!( result, Err(ProposerPreferencesError::InvalidProposalEpoch { .. }) @@ -203,10 +224,13 @@ mod tests { #[test] fn test_proposal_slot_already_passed() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } let current_slot = Slot::new(10); let prefs = make_preferences(Slot::new(9), 0); - let result = verify_preferences_consistency::(&prefs, current_slot, &state()); + let result = verify_preferences_consistency::(&prefs, current_slot, &state(), &spec()); assert!(matches!( result, Err(ProposerPreferencesError::ProposalSlotAlreadyPassed { .. }) @@ -215,10 +239,13 @@ mod tests { #[test] fn test_proposal_slot_equal_to_current() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } let current_slot = Slot::new(10); let prefs = make_preferences(Slot::new(10), 0); - let result = verify_preferences_consistency::(&prefs, current_slot, &state()); + let result = verify_preferences_consistency::(&prefs, current_slot, &state(), &spec()); assert!(matches!( result, Err(ProposerPreferencesError::ProposalSlotAlreadyPassed { .. }) diff --git a/beacon_node/beacon_chain/src/proposer_preferences_verification/mod.rs b/beacon_node/beacon_chain/src/proposer_preferences_verification/mod.rs index a2e96dfce1..6c79e56733 100644 --- a/beacon_node/beacon_chain/src/proposer_preferences_verification/mod.rs +++ b/beacon_node/beacon_chain/src/proposer_preferences_verification/mod.rs @@ -24,7 +24,7 @@ mod tests; #[derive(Debug)] pub enum ProposerPreferencesError { - /// The proposal slot is not in the current or next epoch. + /// The proposal slot is not within the proposer lookahead. InvalidProposalEpoch { proposal_epoch: Epoch }, /// The proposal slot has already passed. ProposalSlotAlreadyPassed { diff --git a/beacon_node/beacon_chain/src/proposer_preferences_verification/proposer_preference_cache.rs b/beacon_node/beacon_chain/src/proposer_preferences_verification/proposer_preference_cache.rs index 7bbdf34888..c423418fbc 100644 --- a/beacon_node/beacon_chain/src/proposer_preferences_verification/proposer_preference_cache.rs +++ b/beacon_node/beacon_chain/src/proposer_preferences_verification/proposer_preference_cache.rs @@ -87,7 +87,7 @@ mod tests { proposal_slot: slot, validator_index, fee_recipient: Address::ZERO, - gas_limit: 30_000_000, + target_gas_limit: 30_000_000, }, signature: Signature::empty(), }), diff --git a/beacon_node/beacon_chain/src/proposer_preferences_verification/tests.rs b/beacon_node/beacon_chain/src/proposer_preferences_verification/tests.rs index 468e08ff3b..53c1c4ded3 100644 --- a/beacon_node/beacon_chain/src/proposer_preferences_verification/tests.rs +++ b/beacon_node/beacon_chain/src/proposer_preferences_verification/tests.rs @@ -112,7 +112,7 @@ impl TestContext { let slot_in_epoch = slot.as_usize() % E::slots_per_epoch() as usize; let epoch = slot.epoch(E::slots_per_epoch()); let current_epoch = state.slot().epoch(E::slots_per_epoch()); - let index = if epoch == current_epoch.saturating_add(1u64) { + let index = if epoch == current_epoch.saturating_add(self.spec.min_seed_lookahead) { E::slots_per_epoch() as usize + slot_in_epoch } else { slot_in_epoch @@ -131,7 +131,7 @@ fn make_signed_preferences( proposal_slot, validator_index, fee_recipient: Address::ZERO, - gas_limit: 30_000_000, + target_gas_limit: 30_000_000, }, signature: Signature::empty(), }) @@ -271,7 +271,7 @@ fn same_validator_different_dependent_root_not_deduplicated() { validator_index: 42, dependent_root: Hash256::repeat_byte(0xaa), fee_recipient: Address::ZERO, - gas_limit: 30_000_000, + target_gas_limit: 30_000_000, }, signature: Signature::empty(), }), diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index be85fc2245..abf1fe48a6 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -1035,6 +1035,7 @@ async fn payload_preparation() { None, None, None, + None, ); assert_eq!(rig.previous_payload_attributes(), payload_attributes); } diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index acf5f2778b..7337a29c8f 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -178,6 +178,8 @@ pub struct PayloadAttributes { pub parent_beacon_block_root: Hash256, #[superstruct(only(V4), partial_getter(copy))] pub slot_number: u64, + #[superstruct(only(V4), partial_getter(copy))] + pub target_gas_limit: u64, } impl PayloadAttributes { @@ -188,19 +190,29 @@ impl PayloadAttributes { withdrawals: Option>, parent_beacon_block_root: Option, slot_number: Option, + target_gas_limit: Option, ) -> Self { - match (withdrawals, parent_beacon_block_root, slot_number) { - (Some(withdrawals), Some(parent_beacon_block_root), Some(slot_number)) => { - PayloadAttributes::V4(PayloadAttributesV4 { - timestamp, - prev_randao, - suggested_fee_recipient, - withdrawals, - parent_beacon_block_root, - slot_number, - }) - } - (Some(withdrawals), Some(parent_beacon_block_root), None) => { + match ( + withdrawals, + parent_beacon_block_root, + slot_number, + target_gas_limit, + ) { + ( + Some(withdrawals), + Some(parent_beacon_block_root), + Some(slot_number), + Some(target_gas_limit), + ) => PayloadAttributes::V4(PayloadAttributesV4 { + timestamp, + prev_randao, + suggested_fee_recipient, + withdrawals, + parent_beacon_block_root, + slot_number, + target_gas_limit, + }), + (Some(withdrawals), Some(parent_beacon_block_root), _, _) => { PayloadAttributes::V3(PayloadAttributesV3 { timestamp, prev_randao, @@ -209,13 +221,13 @@ impl PayloadAttributes { parent_beacon_block_root, }) } - (Some(withdrawals), None, _) => PayloadAttributes::V2(PayloadAttributesV2 { + (Some(withdrawals), None, _, _) => PayloadAttributes::V2(PayloadAttributesV2 { timestamp, prev_randao, suggested_fee_recipient, withdrawals, }), - (None, _, _) => PayloadAttributes::V1(PayloadAttributesV1 { + (None, _, _, _) => PayloadAttributes::V1(PayloadAttributesV1 { timestamp, prev_randao, suggested_fee_recipient, @@ -260,7 +272,7 @@ impl From for SsePayloadAttributes { withdrawals, parent_beacon_block_root, }), - // V4 maps to V3 for SSE (slot_number is not part of the SSE spec) + // V4 maps to V3 for SSE (slot_number/target_gas_limit are not part of the SSE spec) PayloadAttributes::V4(PayloadAttributesV4 { timestamp, prev_randao, @@ -268,6 +280,7 @@ impl From for SsePayloadAttributes { withdrawals, parent_beacon_block_root, slot_number: _, + target_gas_limit: _, }) => Self::V3(SsePayloadAttributesV3 { timestamp, prev_randao, diff --git a/beacon_node/execution_layer/src/engine_api/json_structures.rs b/beacon_node/execution_layer/src/engine_api/json_structures.rs index 9d9391a1e1..fb516e3e16 100644 --- a/beacon_node/execution_layer/src/engine_api/json_structures.rs +++ b/beacon_node/execution_layer/src/engine_api/json_structures.rs @@ -777,6 +777,9 @@ pub struct JsonPayloadAttributes { #[superstruct(only(V4))] #[serde(with = "serde_utils::u64_hex_be")] pub slot_number: u64, + #[superstruct(only(V4))] + #[serde(with = "serde_utils::u64_hex_be")] + pub target_gas_limit: u64, } impl From for JsonPayloadAttributes { @@ -807,6 +810,7 @@ impl From for JsonPayloadAttributes { withdrawals: pa.withdrawals.into_iter().map(Into::into).collect(), parent_beacon_block_root: pa.parent_beacon_block_root, slot_number: pa.slot_number, + target_gas_limit: pa.target_gas_limit, }), } } @@ -840,6 +844,7 @@ impl From for PayloadAttributes { withdrawals: jpa.withdrawals.into_iter().map(Into::into).collect(), parent_beacon_block_root: jpa.parent_beacon_block_root, slot_number: jpa.slot_number, + target_gas_limit: jpa.target_gas_limit, }), } } diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index b2dabb7c01..b1b8b0deaa 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -73,6 +73,8 @@ pub const DEFAULT_EXECUTION_ENDPOINT: &str = "http://localhost:8551/"; /// Name for the default file used for the jwt secret. pub const DEFAULT_JWT_FILE: &str = "jwt.hex"; +pub const DEFAULT_GAS_LIMIT: u64 = 60_000_000; + /// A fee recipient address for use during block production. Only used as a very last resort if /// there is no address provided by the user. /// @@ -358,7 +360,10 @@ impl> BlockProposalContents { pub parent_hash: ExecutionBlockHash, - pub parent_gas_limit: u64, + // NOTE: The `parent_gas_limit` is a bit scuffed. We made it optional for Gloas because it + // isn't currently required, but it should possibly be made non-optional again if needed. + // Or we should superstruct this type. + pub parent_gas_limit: Option, pub proposer_gas_limit: Option, pub payload_attributes: &'a PayloadAttributes, pub forkchoice_update_params: &'a ForkchoiceUpdateParameters, @@ -2082,7 +2087,7 @@ fn verify_builder_bid( let payload_withdrawals_root = header.withdrawals_root().ok(); let expected_gas_limit = proposer_gas_limit - .and_then(|target_gas_limit| expected_gas_limit(parent_gas_limit, target_gas_limit, spec)); + .and_then(|target_gas_limit| expected_gas_limit(parent_gas_limit?, target_gas_limit, spec)); if header.parent_hash() != parent_hash { Err(Box::new(InvalidBuilderPayload::ParentHash { diff --git a/beacon_node/execution_layer/src/test_utils/mock_builder.rs b/beacon_node/execution_layer/src/test_utils/mock_builder.rs index d6243a7c4d..d456c9adc1 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_builder.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_builder.rs @@ -282,7 +282,7 @@ impl BidStuff for BuilderBid { #[derive(Clone)] pub struct PayloadParametersCloned { pub parent_hash: ExecutionBlockHash, - pub parent_gas_limit: u64, + pub parent_gas_limit: Option, pub proposer_gas_limit: Option, pub payload_attributes: PayloadAttributes, pub forkchoice_update_params: ForkchoiceUpdateParameters, @@ -903,6 +903,7 @@ impl MockBuilder { expected_withdrawals, None, None, + None, ), ForkName::Deneb | ForkName::Electra | ForkName::Fulu => PayloadAttributes::new( timestamp, @@ -911,6 +912,7 @@ impl MockBuilder { expected_withdrawals, Some(head_block_root), None, + None, ), ForkName::Gloas => PayloadAttributes::new( timestamp, @@ -919,6 +921,7 @@ impl MockBuilder { expected_withdrawals, Some(head_block_root), Some(slot.as_u64()), + None, // TODO(gloas): pass target_gas_limit ), ForkName::Base | ForkName::Altair => { return Err("invalid fork".to_string()); @@ -969,7 +972,7 @@ impl MockBuilder { let payload_parameters = PayloadParametersCloned { parent_hash: head_execution_hash, - parent_gas_limit: head_gas_limit, + parent_gas_limit: Some(head_gas_limit), proposer_gas_limit: Some(proposer_gas_limit), payload_attributes, forkchoice_update_params, diff --git a/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs b/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs index 5b721bcab2..583808281f 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs @@ -105,6 +105,7 @@ impl MockExecutionLayer { None, None, None, + None, ); // Insert a proposer to ensure the fork choice updated command works. @@ -146,11 +147,12 @@ impl MockExecutionLayer { None, None, None, + None, ); let payload_parameters = PayloadParameters { parent_hash, - parent_gas_limit, + parent_gas_limit: Some(parent_gas_limit), proposer_gas_limit: None, payload_attributes: &payload_attributes, forkchoice_update_params: &forkchoice_update_params, @@ -199,11 +201,12 @@ impl MockExecutionLayer { None, None, None, + None, ); let payload_parameters = PayloadParameters { parent_hash, - parent_gas_limit, + parent_gas_limit: Some(parent_gas_limit), proposer_gas_limit: None, payload_attributes: &payload_attributes, forkchoice_update_params: &forkchoice_update_params, diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index a7fe34593a..3da0841a4e 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -2929,7 +2929,7 @@ impl ApiTester { proposal_slot, validator_index: validator_index as u64, fee_recipient: Address::repeat_byte(0xaa), - gas_limit: 30_000_000, + target_gas_limit: 30_000_000, }; let epoch = proposal_slot.epoch(E::slots_per_epoch()); 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 7a902649cb..3e8845f017 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -4058,7 +4058,6 @@ impl NetworkBeaconProcessor { PayloadBidError::BadSignature | PayloadBidError::InvalidBuilder { .. } | PayloadBidError::InvalidFeeRecipient - | PayloadBidError::InvalidGasLimit | PayloadBidError::ExecutionPaymentNonZero { .. } | PayloadBidError::InvalidBlobKzgCommitments { .. }, ) => { @@ -4076,6 +4075,7 @@ impl NetworkBeaconProcessor { | PayloadBidError::ParentBlockRootUnknown { .. } | PayloadBidError::ParentBlockRootNotCanonical { .. } | PayloadBidError::BuilderCantCoverBid { .. } + | PayloadBidError::InvalidGasLimit | PayloadBidError::BeaconStateError(_) | PayloadBidError::InternalError(_) | PayloadBidError::InvalidBidSlot { .. } diff --git a/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml b/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml index 02bf37cb55..ced9679142 100644 --- a/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml +++ b/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml @@ -93,8 +93,8 @@ SYNC_MESSAGE_DUE_BPS: 3333 CONTRIBUTION_DUE_BPS: 6667 # Gloas -# 2**6 (= 64) epochs -MIN_BUILDER_WITHDRAWABILITY_DELAY: 64 +# 2**13 (= 8192) epochs +MIN_BUILDER_WITHDRAWABILITY_DELAY: 8192 # 2500 basis points, 25% of SLOT_DURATION_MS ATTESTATION_DUE_BPS_GLOAS: 2500 # 5000 basis points, 50% of SLOT_DURATION_MS diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index a60859585c..2de8ce7d81 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1207,7 +1207,6 @@ where fn validate_on_payload_attestation( &self, indexed_payload_attestation: &IndexedPayloadAttestation, - is_from_block: AttestationFromBlock, ) -> Result<(), InvalidPayloadAttestation> { // This check is from `is_valid_indexed_payload_attestation`, but we do it immediately to // avoid wasting time on junk attestations. @@ -1233,25 +1232,6 @@ where }); } - // PTC votes can only change the vote for their assigned beacon block, return early otherwise - if block.slot != indexed_payload_attestation.data.slot { - return Ok(()); - } - - // Gossip payload attestations must be for the current slot. - // NOTE: signature is assumed to have been verified by caller. - // https://github.com/ethereum/consensus-specs/blob/master/specs/gloas/fork-choice.md - if matches!(is_from_block, AttestationFromBlock::False) - && indexed_payload_attestation.data.slot != self.fc_store.get_current_slot() - { - return Err( - InvalidPayloadAttestation::PayloadAttestationNotCurrentSlot { - attestation_slot: indexed_payload_attestation.data.slot, - current_slot: self.fc_store.get_current_slot(), - }, - ); - } - Ok(()) } @@ -1339,34 +1319,69 @@ where pub fn on_payload_attestation( &mut self, system_time_current_slot: Slot, - attestation: &IndexedPayloadAttestation, + payload_attestation: &IndexedPayloadAttestation, is_from_block: AttestationFromBlock, ptc: &[usize], ) -> Result<(), Error> { self.update_time(system_time_current_slot)?; - if attestation.data.beacon_block_root.is_zero() { + if payload_attestation.data.beacon_block_root.is_zero() { return Ok(()); } // TODO(gloas): Should ignore wrong-slot payload attestations at the caller, they could // have been processed at the correct slot when received on gossip, but then have the // wrong-slot by the time they make it to here (TOCTOU). - self.validate_on_payload_attestation(attestation, is_from_block)?; + // TODO(gloas): Consider inlining validate_on_payload_attestation here to look more like the spec. + self.validate_on_payload_attestation(payload_attestation)?; - // Resolve validator indices to PTC committee positions. - let ptc_indices: Vec = attestation - .attesting_indices - .iter() - .filter_map(|validator_index| ptc.iter().position(|&p| p == *validator_index as usize)) - .collect(); + // PTC votes can only change the vote for their assigned beacon block, return early otherwise. + let block = self + .proto_array + .get_block(&payload_attestation.data.beacon_block_root) + .ok_or(InvalidPayloadAttestation::UnknownHeadBlock { + beacon_block_root: payload_attestation.data.beacon_block_root, + })?; + if block.slot != payload_attestation.data.slot { + return Ok(()); + } + + // Gossip payload attestations must be for the current slot. + if matches!(is_from_block, AttestationFromBlock::False) + && payload_attestation.data.slot != self.fc_store.get_current_slot() + { + return Err( + InvalidPayloadAttestation::PayloadAttestationNotCurrentSlot { + attestation_slot: payload_attestation.data.slot, + current_slot: self.fc_store.get_current_slot(), + } + .into(), + ); + } + + // Resolve validator indices to all PTC committee positions. A validator may + // appear multiple times in the PTC committee. + let mut ptc_indices = vec![]; + let mut validators_found = 0; + for validator_index in payload_attestation.attesting_indices.iter() { + let mut found = false; + for (ptc_index, &ptc_validator_index) in ptc.iter().enumerate() { + if ptc_validator_index == *validator_index as usize { + ptc_indices.push(ptc_index); + found = true; + } + } + if found { + validators_found += 1; + } + } // Check that all the attesters are in the PTC - if ptc_indices.len() != attestation.attesting_indices.len() { + if validators_found != payload_attestation.attesting_indices.len() { return Err( InvalidPayloadAttestation::PayloadAttestationAttestersNotInPtc { - attesting_indices_len: attestation.attesting_indices.len(), - attesting_indices_in_ptc: ptc_indices.len(), + attesting_indices_len: payload_attestation.attesting_indices.len(), + attesting_indices_in_ptc: validators_found, } .into(), ); @@ -1374,10 +1389,10 @@ where for &ptc_index in &ptc_indices { self.proto_array.process_payload_attestation( - attestation.data.beacon_block_root, + payload_attestation.data.beacon_block_root, ptc_index, - attestation.data.payload_present, - attestation.data.blob_data_available, + payload_attestation.data.payload_present, + payload_attestation.data.blob_data_available, )?; } @@ -1522,6 +1537,17 @@ where && self.is_finalized_checkpoint_or_descendant(*block_root) } + /// Called by the proposer to decide whether to build on the full or empty parent. + pub fn should_build_on_full( + &self, + block_root: &Hash256, + parent_payload_status: PayloadStatus, + ) -> Result> { + self.proto_array + .should_build_on_full::(block_root, parent_payload_status) + .map_err(Error::ProtoArrayStringError) + } + /// Returns whether the proposer should extend the execution payload chain of the given block. pub fn should_extend_payload(&self, block_root: &Hash256) -> Result> { let proposer_boost_root = self.fc_store.proposer_boost_root(); diff --git a/consensus/proto_array/src/error.rs b/consensus/proto_array/src/error.rs index bb47af97d9..d185ed371c 100644 --- a/consensus/proto_array/src/error.rs +++ b/consensus/proto_array/src/error.rs @@ -1,3 +1,4 @@ +use crate::PayloadStatus; use safe_arith::ArithError; use types::{Checkpoint, Epoch, ExecutionBlockHash, Hash256, Slot}; @@ -62,6 +63,10 @@ pub enum Error { }, NoViableChildren, OnBlockRequiresProposerIndex, + InvalidPayloadStatus { + block_root: Hash256, + payload_status: PayloadStatus, + }, } impl From for Error { diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index d537f16bb2..43b76ec7cb 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -556,7 +556,11 @@ impl ForkChoiceTestDefinition { node_v29.payload_data_availability_votes = BitVector::from_bytes(smallvec::smallvec![fill; 64]) .expect("valid 512-bit bitvector"); - // Per spec, is_payload_timely/is_payload_data_available require + // Mark all PTC members as having participated. + node_v29.ptc_participation = + BitVector::from_bytes(smallvec::smallvec![0xFF; 64]) + .expect("valid 512-bit bitvector"); + // Per spec, payload_timeliness/payload_data_availability require // the payload to be in payload_states (payload_received). node_v29.payload_received = is_timely || is_data_available; } diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 78f5026689..8ac8354f06 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -155,6 +155,10 @@ pub struct ProtoNode { /// Tiebreak derived as: `num_set_bits() > ptc_size / 2`. #[superstruct(only(V29))] pub payload_data_availability_votes: BitVector, + /// Tracks which PTC members have cast a vote. + /// Bit i set means PTC member i has submitted a payload attestation. + #[superstruct(only(V29))] + pub ptc_participation: BitVector, /// Whether the execution payload for this block has been received and validated locally. /// Maps to `root in store.payload_states` in the spec. #[superstruct(only(V29), partial_getter(copy))] @@ -193,31 +197,60 @@ impl ProtoNode { } } - pub fn is_payload_timely(&self) -> bool { + /// Checks if `timely` matches our view of payload timeliness. + /// Returns whether the execution payload for the node is considered `timely` + /// (or not `timely` when `timely` is `false`), taking into consideration local + /// availability and PTC votes. + pub fn payload_timeliness(&self, timely: bool) -> Result { let Ok(node) = self.as_v29() else { - return false; + return Err(Error::InvalidNodeVariant { + block_root: self.root(), + }); }; - // Equivalent to `if root not in store.payload_states` in the spec. + // Equivalent to `if not is_payload_verified(store, root)` in the spec. if !node.payload_received { - return false; + return Ok(!timely); } - node.payload_timeliness_votes.num_set_bits() > E::payload_timely_threshold() + let matching_votes = if timely { + node.payload_timeliness_votes.num_set_bits() + } else { + // We take into consideration only participating ptc votes. An unset bit + // in `payload_timeliness_votes` could be an absent vote or a no vote. + node.ptc_participation + .num_set_bits() + .saturating_sub(node.payload_timeliness_votes.num_set_bits()) + }; + Ok(matching_votes > E::payload_timely_threshold()) } - pub fn is_payload_data_available(&self) -> bool { + /// Checks if `available` matches our view of payload data availability. + /// Return whether the blob data for the node is considered `available` + /// (or not, when `available` is `False`), taking into consideration local + /// availability and PTC votes. + pub fn payload_data_availability(&self, available: bool) -> Result { let Ok(node) = self.as_v29() else { - return false; + return Err(Error::InvalidNodeVariant { + block_root: self.root(), + }); }; - // Equivalent to `if root not in store.payload_states` in the spec. + // Equivalent to `if not is_payload_verified(store, root)` in the spec. if !node.payload_received { - return false; + return Ok(!available); } - node.payload_data_availability_votes.num_set_bits() - > E::data_availability_timely_threshold() + let matching_votes = if available { + node.payload_data_availability_votes.num_set_bits() + } else { + // We take into consideration only participating ptc votes. An unset bit + // in `payload_data_availability_votes` could be an absent vote or a no vote. + node.ptc_participation + .num_set_bits() + .saturating_sub(node.payload_data_availability_votes.num_set_bits()) + }; + Ok(matching_votes > E::data_availability_timely_threshold()) } } @@ -605,6 +638,7 @@ impl ProtoArray { execution_payload_parent_hash, payload_timeliness_votes: BitVector::default(), payload_data_availability_votes: BitVector::default(), + ptc_participation: BitVector::default(), payload_received: false, proposer_index, // Spec: `record_block_timeliness` + `get_forkchoice_store`. @@ -1501,12 +1535,46 @@ impl ProtoArray { } } + /// Called by the proposer to decide whether to build on the full or empty + /// parent pending node. Returns false if the PTC has voted the data as unavailable. + pub fn should_build_on_full( + &self, + fc_node: &IndexedForkChoiceNode, + proto_node: &ProtoNode, + ) -> Result { + if fc_node.payload_status == PayloadStatus::Pending { + return Err(Error::InvalidPayloadStatus { + block_root: proto_node.root(), + payload_status: fc_node.payload_status, + }); + } + + if fc_node.payload_status == PayloadStatus::Empty { + return Ok(false); + } + // Check that false votes have not achieved an absolute majority. This allows the payload to be + // considered available when either a majority have voted true or not enough votes have + // been cast either way. + Ok(!proto_node.payload_data_availability::(false)?) + } + pub fn should_extend_payload( &self, fc_node: &IndexedForkChoiceNode, proto_node: &ProtoNode, proposer_boost_root: Hash256, ) -> Result { + let Ok(node) = proto_node.as_v29() else { + return Err(Error::InvalidNodeVariant { + block_root: fc_node.root, + }); + }; + + // Spec equivalent to `if not is_payload_verified(store, root): return False` + if !node.payload_received { + return Ok(false); + } + // Per spec: `proposer_root == Root()` is one of the `or` conditions that // makes `should_extend_payload` return True. if proposer_boost_root.is_zero() { @@ -1531,11 +1599,10 @@ impl ProtoArray { .ok_or(Error::InvalidNodeIndex(parent_index))? .root(); - Ok( - (proto_node.is_payload_timely::() && proto_node.is_payload_data_available::()) - || proposer_boost_parent_root != fc_node.root - || proposer_boost_node.is_parent_node_full(), - ) + Ok((proto_node.payload_timeliness::(true)? + && proto_node.payload_data_availability::(true)?) + || proposer_boost_parent_root != fc_node.root + || proposer_boost_node.is_parent_node_full()) } /// Update the tree with new finalization information. The tree is only actually pruned if both diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 7abba8a1f6..96d2302266 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -640,6 +640,9 @@ impl ProtoArrayForkChoice { .map_err(|e| { format!("process_payload_attestation: data availability set failed: {e:?}") })?; + v29.ptc_participation + .set(ptc_index, true) + .map_err(|e| format!("process_payload_attestation: participation set failed: {e:?}"))?; Ok(()) } @@ -1006,6 +1009,33 @@ impl ProtoArrayForkChoice { }) } + /// Called by the proposer to decide whether to build on the full or empty + /// parent. Returns false if the PTC has voted the data as unavailable. + pub fn should_build_on_full( + &self, + block_root: &Hash256, + parent_payload_status: PayloadStatus, + ) -> Result { + let block_index = self + .proto_array + .indices + .get(block_root) + .ok_or_else(|| format!("Unknown block root: {block_root:?}"))?; + let proto_node = self + .proto_array + .nodes + .get(*block_index) + .ok_or_else(|| format!("Missing node at index: {block_index}"))?; + let fc_node = IndexedForkChoiceNode { + root: proto_node.root(), + proto_node_index: *block_index, + payload_status: parent_payload_status, + }; + self.proto_array + .should_build_on_full::(&fc_node, proto_node) + .map_err(|e| format!("{e:?}")) + } + /// Returns whether the proposer should extend the parent's execution payload chain. /// /// This checks timeliness, data availability, and proposer boost conditions per the spec. diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index 422e0afe06..f88a325d4e 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -916,25 +916,24 @@ pub fn process_deposit_requests_post_gloas( /// Check if there is a pending deposit for a new validator with the given pubkey. // TODO(gloas): cache the deposit signature validation or remove this loop entirely if possible, // it is `O(n * m)` where `n` is max 8192 and `m` is max 128M. -fn is_pending_validator( - state: &BeaconState, +pub fn is_pending_validator<'a>( + pending_deposits: impl IntoIterator, pubkey: &PublicKeyBytes, spec: &ChainSpec, -) -> Result { - for deposit in state.pending_deposits()?.iter() { - if deposit.pubkey == *pubkey { - let deposit_data = DepositData { - pubkey: deposit.pubkey, - withdrawal_credentials: deposit.withdrawal_credentials, - amount: deposit.amount, - signature: deposit.signature.clone(), - }; - if is_valid_deposit_signature(&deposit_data, spec).is_ok() { - return Ok(true); - } - } - } - Ok(false) +) -> bool { + pending_deposits.into_iter().any(|deposit| { + deposit.pubkey == *pubkey + && is_valid_deposit_signature( + &DepositData { + pubkey: deposit.pubkey, + withdrawal_credentials: deposit.withdrawal_credentials, + amount: deposit.amount, + signature: deposit.signature.clone(), + }, + spec, + ) + .is_ok() + }) } pub fn process_deposit_request_post_gloas( @@ -964,7 +963,7 @@ pub fn process_deposit_request_post_gloas( if is_builder || (has_builder_prefix && !is_validator - && !is_pending_validator(state, &deposit_request.pubkey, spec)?) + && !is_pending_validator(state.pending_deposits()?, &deposit_request.pubkey, spec)) { // Apply builder deposits immediately apply_deposit_for_builder( @@ -1003,7 +1002,7 @@ pub fn apply_deposit_for_builder( signature: SignatureBytes, slot: Slot, spec: &ChainSpec, -) -> Result<(), BeaconStateError> { +) -> Result, BeaconStateError> { match builder_index_opt { None => { // Verify the deposit signature (proof of possession) which is not checked by the deposit contract @@ -1014,13 +1013,16 @@ pub fn apply_deposit_for_builder( signature, }; if is_valid_deposit_signature(&deposit_data, spec).is_ok() { - state.add_builder_to_registry( + let builder_index = state.add_builder_to_registry( pubkey, withdrawal_credentials, amount, slot, spec, )?; + Ok(Some(builder_index)) + } else { + Ok(None) } } Some(builder_index) => { @@ -1030,9 +1032,9 @@ pub fn apply_deposit_for_builder( .ok_or(BeaconStateError::UnknownBuilder(builder_index))? .balance .safe_add_assign(amount)?; + Ok(Some(builder_index)) } } - Ok(()) } // Make sure to build the pubkey cache before calling this function diff --git a/consensus/state_processing/src/upgrade/gloas.rs b/consensus/state_processing/src/upgrade/gloas.rs index 84cdbf22c2..c26547e304 100644 --- a/consensus/state_processing/src/upgrade/gloas.rs +++ b/consensus/state_processing/src/upgrade/gloas.rs @@ -1,18 +1,16 @@ -use crate::per_block_processing::{ - is_valid_deposit_signature, process_operations::apply_deposit_for_builder, -}; +use crate::per_block_processing::process_operations::apply_deposit_for_builder; +use crate::per_block_processing::process_operations::is_pending_validator; use milhouse::{List, Vector}; use safe_arith::SafeArith; use ssz_types::BitVector; use ssz_types::FixedVector; -use std::collections::HashSet; +use std::collections::HashMap; use std::mem; use tree_hash::TreeHash; use typenum::Unsigned; use types::{ BeaconState, BeaconStateError as Error, BeaconStateGloas, BuilderPendingPayment, ChainSpec, - DepositData, EthSpec, ExecutionPayloadBid, ExecutionRequests, Fork, - is_builder_withdrawal_credential, + EthSpec, ExecutionPayloadBid, ExecutionRequests, Fork, is_builder_withdrawal_credential, }; /// Transform a `Fulu` state into a `Gloas` state. @@ -80,6 +78,7 @@ pub fn upgrade_state_to_gloas( // Execution Bid latest_execution_payload_bid: ExecutionPayloadBid { block_hash: pre.latest_execution_payload_header.block_hash, + gas_limit: pre.latest_execution_payload_header.gas_limit, execution_requests_root: ExecutionRequests::::default().tree_hash_root(), ..Default::default() }, @@ -167,66 +166,57 @@ fn onboard_builders_from_pending_deposits( state: &mut BeaconState, spec: &ChainSpec, ) -> Result<(), Error> { - // Rather than tracking all `validator_pubkeys` in one place as the spec does, we keep a - // hashset for *just* the new validator pubkeys, and use the state's efficient - // `get_validator_index` function instead of an O(n) iteration over the full validator list. - let mut new_validator_pubkeys = HashSet::new(); - // Clone pending deposits to avoid borrow conflicts when mutating state. let current_pending_deposits = state.pending_deposits()?.clone(); let mut pending_deposits = List::empty(); + // TODO(gloas): introduce a global builder pubkey cache, see: + // https://github.com/sigp/lighthouse/issues/8783 + let mut builder_pubkey_to_index = state + .builders()? + .iter() + .enumerate() + .map(|(i, b)| (b.pubkey, i as u64)) + .collect::>(); + for deposit in ¤t_pending_deposits { // Deposits for existing validators stay in the pending queue. - if new_validator_pubkeys.contains(&deposit.pubkey) - || state.get_validator_index(&deposit.pubkey)?.is_some() - { + if state.get_validator_index(&deposit.pubkey)?.is_some() { pending_deposits.push(deposit.clone())?; continue; } - // Re-scan builder list each iteration because `apply_deposit_for_builder` may add - // new builders to the registry. - // TODO(gloas): this linear scan could be optimized, see: - // https://github.com/sigp/lighthouse/issues/8783 - let builder_index = state - .builders()? - .iter() - .position(|b| b.pubkey == deposit.pubkey); + if !builder_pubkey_to_index.contains_key(&deposit.pubkey) { + // Deposits without builder withdrawal credentials are for new validators. + if !is_builder_withdrawal_credential(deposit.withdrawal_credentials, spec) { + pending_deposits.push(deposit.clone())?; + continue; + } - let has_builder_credentials = - is_builder_withdrawal_credential(deposit.withdrawal_credentials, spec); - - if builder_index.is_some() || has_builder_credentials { - let builder_index_opt = builder_index.map(|i| i as u64); - apply_deposit_for_builder( - state, - builder_index_opt, - deposit.pubkey, - deposit.withdrawal_credentials, - deposit.amount, - deposit.signature.clone(), - deposit.slot, - spec, - )?; - continue; + // If there is a valid pending deposit for a new validator with this pubkey, + // keep this deposit in the pending queue to be applied to that validator later. + if is_pending_validator(&pending_deposits, &deposit.pubkey, spec) { + pending_deposits.push(deposit.clone())?; + continue; + } } - // If there is a pending deposit for a new validator that has a valid signature, - // track the pubkey so that subsequent builder deposits for the same pubkey stay - // in pending (applied to the validator later) rather than creating a builder. - // Deposits with invalid signatures are dropped since they would fail in - // apply_pending_deposit anyway. - let deposit_data = DepositData { - pubkey: deposit.pubkey, - withdrawal_credentials: deposit.withdrawal_credentials, - amount: deposit.amount, - signature: deposit.signature.clone(), - }; - if is_valid_deposit_signature(&deposit_data, spec).is_ok() { - new_validator_pubkeys.insert(deposit.pubkey); - pending_deposits.push(deposit.clone())?; + let builder_index = builder_pubkey_to_index.get(&deposit.pubkey).copied(); + + if let Some(new_builder_index) = apply_deposit_for_builder( + state, + builder_index, + deposit.pubkey, + deposit.withdrawal_credentials, + deposit.amount, + deposit.signature.clone(), + deposit.slot, + spec, + )? { + builder_pubkey_to_index + .entry(deposit.pubkey) + .or_insert(new_builder_index); } } diff --git a/consensus/types/configs/mainnet.yaml b/consensus/types/configs/mainnet.yaml index 25bf872a7a..743384bcc9 100644 --- a/consensus/types/configs/mainnet.yaml +++ b/consensus/types/configs/mainnet.yaml @@ -91,8 +91,8 @@ SYNC_MESSAGE_DUE_BPS: 3333 CONTRIBUTION_DUE_BPS: 6667 # Gloas -# 2**6 (= 64) epochs -MIN_BUILDER_WITHDRAWABILITY_DELAY: 64 +# 2**13 (= 8192) epochs +MIN_BUILDER_WITHDRAWABILITY_DELAY: 8192 # 2500 basis points, 25% of SLOT_DURATION_MS ATTESTATION_DUE_BPS_GLOAS: 2500 # 5000 basis points, 50% of SLOT_DURATION_MS diff --git a/consensus/types/src/builder/proposer_preferences.rs b/consensus/types/src/builder/proposer_preferences.rs index e3773e333d..4f27020105 100644 --- a/consensus/types/src/builder/proposer_preferences.rs +++ b/consensus/types/src/builder/proposer_preferences.rs @@ -16,7 +16,7 @@ pub struct ProposerPreferences { pub proposal_slot: Slot, pub validator_index: u64, pub fee_recipient: Address, - pub gas_limit: u64, + pub target_gas_limit: u64, } impl SignedRoot for ProposerPreferences {} diff --git a/consensus/types/src/core/chain_spec.rs b/consensus/types/src/core/chain_spec.rs index c54d032891..c42bb4b5b9 100644 --- a/consensus/types/src/core/chain_spec.rs +++ b/consensus/types/src/core/chain_spec.rs @@ -108,6 +108,7 @@ pub struct ChainSpec { pub proposer_reorg_cutoff_bps: u64, pub attestation_due_bps: u64, pub attestation_due_bps_gloas: u64, + pub payload_due_bps: u64, pub payload_attestation_due_bps: u64, pub aggregate_due_bps: u64, pub sync_message_due_bps: u64, @@ -118,6 +119,7 @@ pub struct ChainSpec { */ pub unaggregated_attestation_due: Duration, pub unaggregated_attestation_due_gloas: Duration, + pub payload_due: Duration, pub payload_attestation_due: Duration, pub aggregate_attestation_due: Duration, pub sync_message_due: Duration, @@ -894,6 +896,11 @@ impl ChainSpec { } } + /// Spec: `get_payload_due_ms`. + pub fn get_payload_due(&self) -> Duration { + self.payload_due + } + /// Spec: `get_payload_attestation_due_ms`. pub fn get_payload_attestation_due(&self) -> Duration { self.payload_attestation_due @@ -974,6 +981,9 @@ impl ChainSpec { self.unaggregated_attestation_due_gloas = self .compute_slot_component_duration(self.attestation_due_bps_gloas) .expect("invalid chain spec: cannot compute unaggregated_attestation_due_gloas"); + self.payload_due = self + .compute_slot_component_duration(self.payload_due_bps) + .expect("invalid chain spec: cannot compute payload_due"); self.payload_attestation_due = self .compute_slot_component_duration(self.payload_attestation_due_bps) .expect("invalid chain spec: cannot compute payload_attestation_due"); @@ -1108,6 +1118,7 @@ impl ChainSpec { proposer_reorg_cutoff_bps: 1667, attestation_due_bps: 3333, attestation_due_bps_gloas: 2500, + payload_due_bps: 7500, payload_attestation_due_bps: 7500, aggregate_due_bps: 6667, sync_message_due_bps: 3333, @@ -1118,6 +1129,7 @@ impl ChainSpec { */ unaggregated_attestation_due: Duration::from_millis(3999), unaggregated_attestation_due_gloas: Duration::from_millis(3000), + payload_due: Duration::from_millis(9000), payload_attestation_due: Duration::from_millis(9000), aggregate_attestation_due: Duration::from_millis(8000), sync_message_due: Duration::from_millis(3999), @@ -1270,7 +1282,7 @@ impl ChainSpec { gloas_fork_epoch: None, builder_payment_threshold_numerator: 6, builder_payment_threshold_denominator: 10, - min_builder_withdrawability_delay: Epoch::new(64), + min_builder_withdrawability_delay: Epoch::new(8192), churn_limit_quotient_gloas: option_wrapper(|| u64::checked_pow(2, 15)) .expect("calculation does not overflow"), consolidation_churn_limit_quotient: option_wrapper(|| u64::checked_pow(2, 16)) @@ -1440,6 +1452,7 @@ impl ChainSpec { */ unaggregated_attestation_due: Duration::from_millis(1999), unaggregated_attestation_due_gloas: Duration::from_millis(1500), + payload_due: Duration::from_millis(4500), payload_attestation_due: Duration::from_millis(4500), aggregate_attestation_due: Duration::from_millis(4000), sync_message_due: Duration::from_millis(1999), @@ -1531,6 +1544,7 @@ impl ChainSpec { proposer_reorg_cutoff_bps: 1667, attestation_due_bps: 3333, attestation_due_bps_gloas: 2500, + payload_due_bps: 7500, payload_attestation_due_bps: 7500, aggregate_due_bps: 6667, @@ -1540,6 +1554,7 @@ impl ChainSpec { */ unaggregated_attestation_due: Duration::from_millis(1666), unaggregated_attestation_due_gloas: Duration::from_millis(1250), + payload_due: Duration::from_millis(3750), payload_attestation_due: Duration::from_millis(3750), aggregate_attestation_due: Duration::from_millis(3333), sync_message_due: Duration::from_millis(1666), @@ -1693,7 +1708,7 @@ impl ChainSpec { gloas_fork_epoch: None, builder_payment_threshold_numerator: 6, builder_payment_threshold_denominator: 10, - min_builder_withdrawability_delay: Epoch::new(64), + min_builder_withdrawability_delay: Epoch::new(8192), churn_limit_quotient_gloas: option_wrapper(|| u64::checked_pow(2, 15)) .expect("calculation does not overflow"), consolidation_churn_limit_quotient: option_wrapper(|| u64::checked_pow(2, 16)) @@ -2136,6 +2151,9 @@ pub struct Config { #[serde(default = "default_attestation_due_bps_gloas")] #[serde(with = "serde_utils::quoted_u64")] attestation_due_bps_gloas: u64, + #[serde(default = "default_payload_due_bps")] + #[serde(with = "serde_utils::quoted_u64")] + payload_due_bps: u64, #[serde(default = "default_payload_attestation_due_bps")] #[serde(with = "serde_utils::quoted_u64")] payload_attestation_due_bps: u64, @@ -2379,6 +2397,10 @@ const fn default_attestation_due_bps_gloas() -> u64 { 2500 } +const fn default_payload_due_bps() -> u64 { + 7500 +} + const fn default_payload_attestation_due_bps() -> u64 { 7500 } @@ -2396,7 +2418,7 @@ const fn default_contribution_due_bps() -> u64 { } const fn default_min_builder_withdrawability_delay() -> u64 { - 64 + 8192 } const fn default_churn_limit_quotient_gloas() -> u64 { @@ -2656,6 +2678,7 @@ impl Config { proposer_reorg_cutoff_bps: spec.proposer_reorg_cutoff_bps, attestation_due_bps: spec.attestation_due_bps, attestation_due_bps_gloas: spec.attestation_due_bps_gloas, + payload_due_bps: spec.payload_due_bps, payload_attestation_due_bps: spec.payload_attestation_due_bps, aggregate_due_bps: spec.aggregate_due_bps, sync_message_due_bps: spec.sync_message_due_bps, @@ -2759,6 +2782,7 @@ impl Config { proposer_reorg_cutoff_bps, attestation_due_bps, attestation_due_bps_gloas, + payload_due_bps, payload_attestation_due_bps, aggregate_due_bps, sync_message_due_bps, @@ -2867,6 +2891,7 @@ impl Config { proposer_reorg_cutoff_bps, attestation_due_bps, attestation_due_bps_gloas, + payload_due_bps, payload_attestation_due_bps, aggregate_due_bps, sync_message_due_bps, @@ -3694,6 +3719,30 @@ mod yaml_tests { let custom_spec = custom_spec.compute_derived_values::(); let tiny_due = custom_spec.get_unaggregated_attestation_due(); assert_eq!(tiny_due, Duration::from_millis(1)); // 12000 * 1 / 10000 = 1.2 -> 1 + + // Test payload due (7500 bps = 75% of 12s = 9s) + let spec = ChainSpec::mainnet().compute_derived_values::(); + let payload_due = spec.get_payload_due(); + assert_eq!(payload_due, Duration::from_millis(9000)); // 12000 * 7500 / 10000 + + // Test payload attestation due (7500 bps = 75% of 12s = 9s) + let payload_att_due = spec.get_payload_attestation_due(); + assert_eq!(payload_att_due, Duration::from_millis(9000)); // 12000 * 7500 / 10000 + + // Test gloas attestation due (2500 bps = 25% of 12s = 3s) + assert_eq!( + spec.unaggregated_attestation_due_gloas, + Duration::from_millis(3000) + ); // 12000 * 2500 / 10000 + + // Test gloas with custom bps + let mut custom_spec = spec; + custom_spec.attestation_due_bps_gloas = 5000; + let custom_spec = custom_spec.compute_derived_values::(); + assert_eq!( + custom_spec.unaggregated_attestation_due_gloas, + Duration::from_millis(6000) + ); // 12000 * 5000 / 10000 } #[test] @@ -3715,6 +3764,19 @@ mod yaml_tests { Duration::from_millis(8000) ); + // Mainnet payload due: 12000ms slots, 7500 bps = 9000ms + assert_eq!(mainnet.get_payload_due(), Duration::from_millis(9000)); + assert_eq!( + mainnet.get_payload_attestation_due(), + Duration::from_millis(9000) + ); + + // Mainnet gloas: 12000ms slots, 2500 bps = 3000ms + assert_eq!( + mainnet.unaggregated_attestation_due_gloas, + Duration::from_millis(3000) + ); + // Minimal spec: 6000ms slots, 3333 bps = 1999ms, 6667 bps = 4000ms let minimal = ChainSpec::minimal(); assert_eq!( @@ -3730,6 +3792,18 @@ mod yaml_tests { minimal.get_contribution_message_due(), Duration::from_millis(4000) ); + // Minimal payload due: 6000ms slots, 7500 bps = 4500ms + assert_eq!(minimal.get_payload_due(), Duration::from_millis(4500)); + assert_eq!( + minimal.get_payload_attestation_due(), + Duration::from_millis(4500) + ); + + // Minimal gloas: 6000ms slots, 2500 bps = 1500ms + assert_eq!( + minimal.unaggregated_attestation_due_gloas, + Duration::from_millis(1500) + ); // Gnosis spec: 5000ms slots, 3333 bps = 1666ms, 6667 bps = 3333ms let gnosis = ChainSpec::gnosis(); @@ -3746,6 +3820,18 @@ mod yaml_tests { gnosis.get_contribution_message_due(), Duration::from_millis(3333) ); + // Gnosis payload due: 5000ms slots, 7500 bps = 3750ms + assert_eq!(gnosis.get_payload_due(), Duration::from_millis(3750)); + assert_eq!( + gnosis.get_payload_attestation_due(), + Duration::from_millis(3750) + ); + + // Gnosis gloas: 5000ms slots, 2500 bps = 1250ms + assert_eq!( + gnosis.unaggregated_attestation_due_gloas, + Duration::from_millis(1250) + ); } #[test] diff --git a/consensus/types/src/state/beacon_state.rs b/consensus/types/src/state/beacon_state.rs index 4d2c7533ca..027acfab7f 100644 --- a/consensus/types/src/state/beacon_state.rs +++ b/consensus/types/src/state/beacon_state.rs @@ -1333,6 +1333,7 @@ impl BeaconState { pub fn is_valid_proposal_slot( &self, preferences: &ProposerPreferences, + spec: &ChainSpec, ) -> Result { let current_epoch = self.current_epoch(); let proposal_epoch = preferences.proposal_slot.epoch(E::slots_per_epoch()); @@ -1341,8 +1342,7 @@ impl BeaconState { return Ok(false); } - let next_epoch = current_epoch.saturating_add(1u64); - if proposal_epoch > next_epoch { + if proposal_epoch > current_epoch.saturating_add(spec.min_seed_lookahead) { return Ok(false); } diff --git a/testing/ef_tests/Makefile b/testing/ef_tests/Makefile index 36f6684685..f566a89ded 100644 --- a/testing/ef_tests/Makefile +++ b/testing/ef_tests/Makefile @@ -1,6 +1,6 @@ # To download/extract nightly tests, run: # CONSENSUS_SPECS_TEST_VERSION=nightly make -CONSENSUS_SPECS_TEST_VERSION ?= v1.7.0-alpha.7 +CONSENSUS_SPECS_TEST_VERSION ?= v1.7.0-alpha.8 REPO_NAME := consensus-spec-tests OUTPUT_DIR := ./$(REPO_NAME) diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 8b0b74d256..69fce09505 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -1,6 +1,6 @@ use super::*; use crate::decode::{ssz_decode_file, ssz_decode_file_with, ssz_decode_state, yaml_decode_file}; -use ::fork_choice::{PayloadVerificationStatus, ProposerHeadError}; +use ::fork_choice::{AttestationFromBlock, PayloadVerificationStatus, ProposerHeadError}; use beacon_chain::beacon_proposer_cache::compute_proposer_duties_from_head; use beacon_chain::blob_verification::GossipBlobError; use beacon_chain::block_verification_types::LookupBlock; @@ -19,13 +19,16 @@ use beacon_chain::{ custody_context::NodeCustodyType, test_utils::{BeaconChainHarness, EphemeralHarnessType}, }; +use bls::AggregateSignature; use execution_layer::{ PayloadStatusV1, PayloadStatusV1Status, json_structures::JsonPayloadStatusV1Status, }; use serde::Deserialize; use ssz_derive::Decode; +use ssz_types::VariableList; use state_processing::VerifySignatures; use state_processing::envelope_processing::verify_execution_payload_envelope; +use state_processing::per_block_processing::is_valid_indexed_payload_attestation; use state_processing::state_advance::complete_state_advance; use std::future::Future; use std::sync::Arc; @@ -34,8 +37,8 @@ use types::{ Attestation, AttestationRef, AttesterSlashing, AttesterSlashingRef, BeaconBlock, BeaconState, BlobSidecar, BlobsList, BlockImportSource, Checkpoint, DataColumnSidecar, DataColumnSidecarList, DataColumnSubnetId, ExecutionBlockHash, Hash256, IndexedAttestation, - KzgProof, ProposerPreparationData, SignedBeaconBlock, SignedExecutionPayloadEnvelope, Slot, - Uint256, + IndexedPayloadAttestation, KzgProof, PayloadAttestationMessage, ProposerPreparationData, + SignedBeaconBlock, SignedExecutionPayloadEnvelope, Slot, Uint256, }; // When set to true, cache any states fetched from the db. @@ -63,6 +66,13 @@ pub struct ShouldOverrideFcu { result: bool, } +#[derive(Debug, Clone, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct PayloadVoteCheck { + block_root: Hash256, + votes: Vec>, +} + #[derive(Debug, Clone, Deserialize)] #[serde(deny_unknown_fields)] pub struct Checks { @@ -78,6 +88,8 @@ pub struct Checks { get_proposer_head: Option, should_override_forkchoice_update: Option, head_payload_status: Option, + payload_timeliness_vote: Option, + payload_data_availability_vote: Option, } #[derive(Debug, Clone, Deserialize)] @@ -108,6 +120,7 @@ pub enum Step< TAttesterSlashing, TPowBlock, TExecutionPayload = String, + TPayloadAttestationMessage = String, > { Tick { tick: u64, @@ -146,6 +159,15 @@ pub enum Step< execution_payload: TExecutionPayload, valid: bool, }, + PayloadAttestationMessage { + payload_attestation_message: TPayloadAttestationMessage, + #[serde(default = "default_true")] + valid: bool, + }, +} + +fn default_true() -> bool { + true } #[derive(Debug, Clone, Deserialize)] @@ -170,6 +192,7 @@ pub struct ForkChoiceTest { AttesterSlashing, PowBlock, SignedExecutionPayloadEnvelope, + PayloadAttestationMessage, >, >, } @@ -184,8 +207,12 @@ impl LoadCase for ForkChoiceTest { .expect("path must be valid OsStr") .to_string(); let spec = &testing_spec::(fork_name); - let steps: Vec, String, String, String>> = - yaml_decode_file(&path.join("steps.yaml"))?; + + #[allow(clippy::type_complexity)] + let steps: Vec< + Step, String, String, String, String, String>, + > = yaml_decode_file(&path.join("steps.yaml"))?; + // Resolve the object names in `steps.yaml` into actual decoded block/attestation objects. let steps = steps .into_iter() @@ -301,6 +328,18 @@ impl LoadCase for ForkChoiceTest { valid, }) } + Step::PayloadAttestationMessage { + payload_attestation_message, + valid, + } => { + let msg: PayloadAttestationMessage = ssz_decode_file( + &path.join(format!("{payload_attestation_message}.ssz_snappy")), + )?; + Ok(Step::PayloadAttestationMessage { + payload_attestation_message: msg, + valid, + }) + } }) .collect::>()?; let anchor_state = ssz_decode_state(&path.join("anchor_state.ssz_snappy"), spec)?; @@ -381,6 +420,8 @@ impl Case for ForkChoiceTest { get_proposer_head, should_override_forkchoice_update: should_override_fcu, head_payload_status, + payload_timeliness_vote, + payload_data_availability_vote, } = checks.as_ref(); if let Some(expected_head) = head { @@ -431,6 +472,14 @@ impl Case for ForkChoiceTest { if let Some(expected_status) = head_payload_status { tester.check_head_payload_status(*expected_status)?; } + + if let Some(expected) = payload_timeliness_vote { + tester.check_payload_timeliness_vote(expected)?; + } + + if let Some(expected) = payload_data_availability_vote { + tester.check_payload_data_availability_vote(expected)?; + } } Step::MaybeValidBlockAndColumns { @@ -446,6 +495,13 @@ impl Case for ForkChoiceTest { } => { tester.process_execution_payload(execution_payload, *valid)?; } + Step::PayloadAttestationMessage { + payload_attestation_message, + valid, + } => { + tester + .process_payload_attestation_message(payload_attestation_message, *valid)?; + } } } @@ -1149,6 +1205,173 @@ impl Tester { expected_should_override_fcu.result, ) } + + pub fn process_payload_attestation_message( + &self, + msg: &PayloadAttestationMessage, + valid: bool, + ) -> Result<(), Error> { + let slot = msg.data.slot; + let block_root = msg.data.beacon_block_root; + + // Get the state at the block to compute the PTC and verify signature. + let store = &self.harness.chain.store; + let block = store + .get_blinded_block(&block_root) + .map_err(|e| Error::InternalError(format!("Failed to load block: {e:?}")))?; + + let state_opt = block.and_then(|block| { + store + .get_hot_state(&block.state_root(), CACHE_STATE_IN_TESTS) + .ok()? + }); + + // Build IndexedPayloadAttestation from the message. + let indexed = IndexedPayloadAttestation:: { + attesting_indices: VariableList::new(vec![msg.validator_index]).unwrap(), + data: msg.data.clone(), + signature: AggregateSignature::from(&msg.signature), + }; + + let result = if let Some(ref state) = state_opt { + is_valid_indexed_payload_attestation( + state, + &indexed, + VerifySignatures::True, + &self.spec, + ) + .map_err(|e| { + Error::InternalError(format!( + "payload attestation signature verification failed for validator {}: {:?}", + msg.validator_index, e + )) + }) + .and_then(|_| { + let ptc = state.get_ptc(slot, &self.spec).map_err(|e| { + Error::InternalError(format!( + "Could not compute PTC for block root {block_root:?} at slot {slot:?}: {e:?}" + )) + })?; + + self.harness + .chain + .canonical_head + .fork_choice_write_lock() + .on_payload_attestation( + self.harness.chain.slot().unwrap(), + &indexed, + AttestationFromBlock::False, + &ptc.0, + ) + .map_err(|e| { + Error::InternalError(format!( + "on_payload_attestation for validator {} failed: {:?}", + msg.validator_index, e + )) + }) + }) + } else { + Err(Error::InternalError(format!( + "Could not get state for block root {block_root:?} at slot {slot:?}" + ))) + }; + + if valid { + result?; + } else if result.is_ok() { + return Err(Error::DidntFail(format!( + "payload_attestation_message for validator {} should have failed", + msg.validator_index + ))); + } + + Ok(()) + } + + pub fn check_payload_timeliness_vote(&self, expected: &PayloadVoteCheck) -> Result<(), Error> { + let fc = self.harness.chain.canonical_head.fork_choice_read_lock(); + let proto_array = fc.proto_array().core_proto_array(); + + let node_index = proto_array + .indices + .get(&expected.block_root) + .ok_or_else(|| { + Error::InternalError(format!( + "Block root {:?} not found in proto array", + expected.block_root + )) + })?; + let node = proto_array + .nodes + .get(*node_index) + .ok_or_else(|| Error::InternalError(format!("Node index {} not found", node_index)))?; + let v29 = node + .as_v29() + .map_err(|_| Error::InternalError("Node is not V29".to_string()))?; + + let timeliness_votes = &v29.payload_timeliness_votes; + let participation = &v29.ptc_participation; + + for (i, expected_vote) in expected.votes.iter().enumerate() { + let actual = if !participation.get(i).unwrap() { + None // not yet voted + } else { + Some(timeliness_votes.get(i).unwrap()) + }; + if actual != *expected_vote { + return Err(Error::NotEqual(format!( + "payload_timeliness_vote[{}]: Got {:?} | Expected {:?}", + i, actual, expected_vote + ))); + } + } + + Ok(()) + } + + pub fn check_payload_data_availability_vote( + &self, + expected: &PayloadVoteCheck, + ) -> Result<(), Error> { + let fc = self.harness.chain.canonical_head.fork_choice_read_lock(); + let proto_array = fc.proto_array().core_proto_array(); + + let node_index = proto_array + .indices + .get(&expected.block_root) + .ok_or_else(|| { + Error::InternalError(format!( + "Block root {:?} not found in proto array", + expected.block_root + )) + })?; + let node = proto_array + .nodes + .get(*node_index) + .ok_or_else(|| Error::InternalError(format!("Node index {} not found", node_index)))?; + let v29 = node + .as_v29() + .map_err(|_| Error::InternalError("Node is not V29".to_string()))?; + + let availability_votes = &v29.payload_data_availability_votes; + let participation = &v29.ptc_participation; + + for (i, expected_vote) in expected.votes.iter().enumerate() { + let actual = if !participation.get(i).unwrap() { + None // not yet voted + } else { + Some(availability_votes.get(i).unwrap()) + }; + if actual != *expected_vote { + return Err(Error::NotEqual(format!( + "payload_data_availability_vote[{}]: Got {:?} | Expected {:?}", + i, actual, expected_vote + ))); + } + } + + Ok(()) + } } /// Checks that the `head` checkpoint from the beacon chain head matches the `fc` checkpoint gleaned diff --git a/testing/ef_tests/src/handler.rs b/testing/ef_tests/src/handler.rs index e380f51c0a..52cc5d57ae 100644 --- a/testing/ef_tests/src/handler.rs +++ b/testing/ef_tests/src/handler.rs @@ -715,10 +715,8 @@ impl Handler for ForkChoiceHandler { return false; } - // Deposit tests exist only for Electra and Fulu (not Gloas). - if self.handler_name == "deposit_with_reorg" - && (!fork_name.electra_enabled() || fork_name.gloas_enabled()) - { + // Deposit tests exist only for Electra and later. + if self.handler_name == "deposit_with_reorg" && !fork_name.electra_enabled() { return false; } @@ -727,10 +725,11 @@ impl Handler for ForkChoiceHandler { return false; } - // on_execution_payload_envelope and get_parent_payload_status tests exist only for - // Gloas and later. + // on_execution_payload_envelope, get_parent_payload_status, and + // on_payload_attestation_message tests exist only for Gloas and later. if (self.handler_name == "on_execution_payload_envelope" - || self.handler_name == "get_parent_payload_status") + || self.handler_name == "get_parent_payload_status" + || self.handler_name == "on_payload_attestation_message") && !fork_name.gloas_enabled() { return false; diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index ca383efdb0..0ff854bd21 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -1079,6 +1079,12 @@ fn fork_choice_get_parent_payload_status() { ForkChoiceHandler::::new("get_parent_payload_status").run(); } +#[test] +fn fork_choice_on_payload_attestation_message() { + ForkChoiceHandler::::new("on_payload_attestation_message").run(); + ForkChoiceHandler::::new("on_payload_attestation_message").run(); +} + #[test] fn optimistic_sync() { OptimisticSyncHandler::::default().run(); diff --git a/testing/execution_engine_integration/src/test_rig.rs b/testing/execution_engine_integration/src/test_rig.rs index ed6b5787b5..61f25e63a1 100644 --- a/testing/execution_engine_integration/src/test_rig.rs +++ b/testing/execution_engine_integration/src/test_rig.rs @@ -320,6 +320,7 @@ impl TestRig { Some(vec![]), None, None, + None, ), ) .await; @@ -366,11 +367,12 @@ impl TestRig { Some(vec![]), None, None, + None, ); let payload_parameters = PayloadParameters { parent_hash, - parent_gas_limit, + parent_gas_limit: Some(parent_gas_limit), proposer_gas_limit: None, payload_attributes: &payload_attributes, forkchoice_update_params: &forkchoice_update_params, @@ -527,11 +529,12 @@ impl TestRig { Some(vec![]), None, None, + None, ); let payload_parameters = PayloadParameters { parent_hash, - parent_gas_limit, + parent_gas_limit: Some(parent_gas_limit), proposer_gas_limit: None, payload_attributes: &payload_attributes, forkchoice_update_params: &forkchoice_update_params, @@ -588,6 +591,7 @@ impl TestRig { Some(vec![]), None, None, + None, ); let slot = Slot::new(42); let head_block_root = Hash256::repeat_byte(100); diff --git a/validator_client/validator_services/src/proposer_preferences_service.rs b/validator_client/validator_services/src/proposer_preferences_service.rs index fbefdf5d96..fc17a1bce6 100644 --- a/validator_client/validator_services/src/proposer_preferences_service.rs +++ b/validator_client/validator_services/src/proposer_preferences_service.rs @@ -136,7 +136,7 @@ impl ProposerPreferencesSer proposal_slot: duty.slot, validator_index: duty.validator_index, fee_recipient, - gas_limit: proposal_data.gas_limit, + target_gas_limit: proposal_data.gas_limit, }, )); }