diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index d3689f70680..3ee8c7ae3f9 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -516,11 +516,7 @@ where self } - pub fn mock_execution_layer(self) -> Self { - self.mock_execution_layer_with_config() - } - - pub fn mock_execution_layer_with_config(mut self) -> Self { + pub fn mock_execution_layer(mut self) -> Self { let mock = mock_execution_layer_from_parts::( self.spec.clone().expect("cannot build without spec"), self.runtime.task_executor.clone(), diff --git a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs index b057abe8877..e01b8de9e30 100644 --- a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs +++ b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs @@ -142,6 +142,7 @@ pub struct ExecutionBlockGenerator { pub pending_payloads: HashMap>, pub next_payload_id: u64, pub payload_ids: HashMap>, + min_blobs_count: usize, /* * Post-merge fork triggers */ @@ -188,6 +189,7 @@ impl ExecutionBlockGenerator { pending_payloads: <_>::default(), next_payload_id: 0, payload_ids: <_>::default(), + min_blobs_count: 0, shanghai_time, cancun_time, prague_time, @@ -318,6 +320,10 @@ impl ExecutionBlockGenerator { Ok(()) } + pub fn set_min_blob_count(&mut self, count: usize) { + self.min_blobs_count = count; + } + pub fn insert_pow_block(&mut self, block_number: u64) -> Result<(), String> { if let Some(finalized_block_hash) = self.finalized_block_hash { return Err(format!( @@ -702,8 +708,10 @@ impl ExecutionBlockGenerator { if fork_name.deneb_enabled() { // get random number between 0 and Max Blobs let mut rng = self.rng.lock(); - let max_blobs = self.spec.max_blobs_per_block_by_fork(fork_name) as usize; - let num_blobs = rng.gen::() % (max_blobs + 1); + // TODO(EIP-7892): see FIXME below + // FIXME: this will break with BPO forks. This function needs to calculate the epoch based on block timestamp.. + let max_blobs = self.spec.max_blobs_per_block_within_fork(fork_name) as usize; + let num_blobs = rng.gen_range(self.min_blobs_count..=max_blobs); let (bundle, transactions) = generate_blobs(num_blobs, fork_name)?; for tx in Vec::from(transactions) { execution_payload diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index a5a21fd985e..4ad70c34671 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -138,7 +138,7 @@ impl ApiTester { .deterministic_keypairs(VALIDATOR_COUNT) .deterministic_withdrawal_keypairs(VALIDATOR_COUNT) .fresh_ephemeral_store() - .mock_execution_layer_with_config() + .mock_execution_layer() .build(); harness diff --git a/beacon_node/lighthouse_network/src/rpc/handler.rs b/beacon_node/lighthouse_network/src/rpc/handler.rs index 33c5521c3bc..8c35bf71459 100644 --- a/beacon_node/lighthouse_network/src/rpc/handler.rs +++ b/beacon_node/lighthouse_network/src/rpc/handler.rs @@ -28,7 +28,7 @@ use std::{ use tokio::time::{sleep, Sleep}; use tokio_util::time::{delay_queue, DelayQueue}; use tracing::{debug, trace}; -use types::{EthSpec, ForkContext}; +use types::{EthSpec, ForkContext, Slot}; /// The number of times to retry an outbound upgrade in the case of IO errors. const IO_ERROR_RETRIES: u8 = 3; @@ -932,9 +932,8 @@ where } } RequestType::BlobsByRange(request) => { - let max_requested_blobs = request - .count - .saturating_mul(spec.max_blobs_per_block_by_fork(current_fork)); + let epoch = Slot::new(request.start_slot).epoch(E::slots_per_epoch()); + let max_requested_blobs = request.max_blobs_requested(epoch, spec); let max_allowed = spec.max_request_blob_sidecars(current_fork) as u64; if max_requested_blobs > max_allowed { self.events_out.push(HandlerEvent::Err(HandlerErr::Inbound { diff --git a/beacon_node/lighthouse_network/src/rpc/methods.rs b/beacon_node/lighthouse_network/src/rpc/methods.rs index 9fe2fef9e89..8a11a6f29d6 100644 --- a/beacon_node/lighthouse_network/src/rpc/methods.rs +++ b/beacon_node/lighthouse_network/src/rpc/methods.rs @@ -16,11 +16,10 @@ use types::blob_sidecar::BlobIdentifier; use types::light_client_update::MAX_REQUEST_LIGHT_CLIENT_UPDATES; use types::{ blob_sidecar::BlobSidecar, ChainSpec, ColumnIndex, DataColumnSidecar, - DataColumnsByRootIdentifier, Epoch, EthSpec, Hash256, LightClientBootstrap, + DataColumnsByRootIdentifier, Epoch, EthSpec, ForkContext, Hash256, LightClientBootstrap, LightClientFinalityUpdate, LightClientOptimisticUpdate, LightClientUpdate, RuntimeVariableList, SignedBeaconBlock, Slot, }; -use types::{ForkContext, ForkName}; /// Maximum length of error message. pub type MaxErrorLen = U256; @@ -328,8 +327,8 @@ pub struct BlobsByRangeRequest { } impl BlobsByRangeRequest { - pub fn max_blobs_requested(&self, current_fork: ForkName, spec: &ChainSpec) -> u64 { - let max_blobs_per_block = spec.max_blobs_per_block_by_fork(current_fork); + pub fn max_blobs_requested(&self, epoch: Epoch, spec: &ChainSpec) -> u64 { + let max_blobs_per_block = spec.max_blobs_per_block(epoch); self.count.saturating_mul(max_blobs_per_block) } } diff --git a/beacon_node/lighthouse_network/src/rpc/protocol.rs b/beacon_node/lighthouse_network/src/rpc/protocol.rs index 820f50ac939..bfe64f58dbf 100644 --- a/beacon_node/lighthouse_network/src/rpc/protocol.rs +++ b/beacon_node/lighthouse_network/src/rpc/protocol.rs @@ -21,7 +21,7 @@ use types::{ EmptyBlock, EthSpec, EthSpecId, ForkContext, ForkName, LightClientBootstrap, LightClientBootstrapAltair, LightClientFinalityUpdate, LightClientFinalityUpdateAltair, LightClientOptimisticUpdate, LightClientOptimisticUpdateAltair, LightClientUpdate, - MainnetEthSpec, MinimalEthSpec, Signature, SignedBeaconBlock, + MainnetEthSpec, MinimalEthSpec, Signature, SignedBeaconBlock, Slot, }; // Note: Hardcoding the `EthSpec` type for `SignedBeaconBlock` as min/max values is @@ -633,7 +633,8 @@ pub fn rpc_blob_limits() -> RpcLimits { pub fn rpc_data_column_limits(fork_name: ForkName, spec: &ChainSpec) -> RpcLimits { RpcLimits::new( DataColumnSidecar::::min_size(), - DataColumnSidecar::::max_size(spec.max_blobs_per_block_by_fork(fork_name) as usize), + // TODO(EIP-7892): fix this once we change fork-version on BPO forks + DataColumnSidecar::::max_size(spec.max_blobs_per_block_within_fork(fork_name) as usize), ) } @@ -732,13 +733,16 @@ impl RequestType { /* These functions are used in the handler for stream management */ /// Maximum number of responses expected for this request. - pub fn max_responses(&self, current_fork: ForkName, spec: &ChainSpec) -> u64 { + /// TODO(EIP-7892): refactor this to remove `_current_fork` + pub fn max_responses(&self, _current_fork: ForkName, spec: &ChainSpec) -> u64 { match self { RequestType::Status(_) => 1, RequestType::Goodbye(_) => 0, RequestType::BlocksByRange(req) => *req.count(), RequestType::BlocksByRoot(req) => req.block_roots().len() as u64, - RequestType::BlobsByRange(req) => req.max_blobs_requested(current_fork, spec), + RequestType::BlobsByRange(req) => { + req.max_blobs_requested(Slot::new(req.start_slot).epoch(E::slots_per_epoch()), spec) + } RequestType::BlobsByRoot(req) => req.blob_ids.len() as u64, RequestType::DataColumnsByRoot(req) => req.max_requested() as u64, RequestType::DataColumnsByRange(req) => req.max_requested::(), diff --git a/beacon_node/network/src/network_beacon_processor/tests.rs b/beacon_node/network/src/network_beacon_processor/tests.rs index 87a5a772941..cb9c9764044 100644 --- a/beacon_node/network/src/network_beacon_processor/tests.rs +++ b/beacon_node/network/src/network_beacon_processor/tests.rs @@ -37,8 +37,8 @@ use tokio::sync::mpsc; use types::blob_sidecar::FixedBlobSidecarList; use types::{ Attestation, AttesterSlashing, BlobSidecar, BlobSidecarList, ChainSpec, DataColumnSidecarList, - DataColumnSubnetId, Epoch, EthSpec, ForkName, Hash256, MainnetEthSpec, ProposerSlashing, - SignedAggregateAndProof, SignedBeaconBlock, SignedVoluntaryExit, Slot, SubnetId, + DataColumnSubnetId, Epoch, Hash256, MainnetEthSpec, ProposerSlashing, SignedAggregateAndProof, + SignedBeaconBlock, SignedVoluntaryExit, Slot, SubnetId, }; type E = MainnetEthSpec; @@ -129,6 +129,14 @@ impl TestRig { "precondition: current slot is one after head" ); + // Ensure there is a blob in the next block. Required for some tests. + harness + .mock_execution_layer + .as_ref() + .unwrap() + .server + .execution_block_generator() + .set_min_blob_count(1); let (next_block_tuple, next_state) = harness .make_block(head.beacon_state.clone(), harness.chain.slot().unwrap()) .await; @@ -799,9 +807,11 @@ async fn import_gossip_block_unacceptably_early() { /// Data columns that have already been processed but unobserved should be propagated without re-importing. #[tokio::test] async fn accept_processed_gossip_data_columns_without_import() { - let processor_config = BeaconProcessorConfig::default(); - let fulu_genesis_spec = ForkName::Fulu.make_genesis_spec(E::default_spec()); - let mut rig = TestRig::new_parametric(SMALL_CHAIN, processor_config, fulu_genesis_spec).await; + if test_spec::().fulu_fork_epoch.is_none() { + return; + }; + + let mut rig = TestRig::new(SMALL_CHAIN).await; // GIVEN the data columns have already been processed but unobserved. // 1. verify data column with `DoNotObserve` to create verified but unobserved data columns. diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index fa3fa047831..52cc91ba298 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -1296,7 +1296,9 @@ impl BeaconNodeHttpClient { } self.get_fork_contextual(path, |fork| { - (fork, spec.max_blobs_per_block_by_fork(fork) as usize) + // TODO(EIP-7892): this will overestimate the max number of blobs + // It would be better if we could get an epoch passed into this function + (fork, spec.max_blobs_per_block_within_fork(fork) as usize) }) .await .map(|opt| opt.map(BeaconResponse::ForkVersioned)) diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 7b9950db915..59472e2edcd 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -243,7 +243,7 @@ pub struct ChainSpec { /* * Networking Fulu */ - max_blobs_per_block_fulu: u64, + blob_schedule: BlobSchedule, /* * Networking Derived @@ -653,19 +653,40 @@ impl ChainSpec { } } - /// Return the value of `MAX_BLOBS_PER_BLOCK` appropriate for the fork at `epoch`. + /// Return the value of `MAX_BLOBS_PER_BLOCK` for the given `epoch`. + /// NOTE: this function is *technically* not spec compliant, but + /// I'm told this is what the other clients are doing for `devnet-0`.. pub fn max_blobs_per_block(&self, epoch: Epoch) -> u64 { - self.max_blobs_per_block_by_fork(self.fork_name_at_epoch(epoch)) + match self.fulu_fork_epoch { + Some(fulu_epoch) if epoch >= fulu_epoch => self + .blob_schedule + .max_blobs_for_epoch(epoch) + .unwrap_or(self.max_blobs_per_block_electra), + _ => match self.electra_fork_epoch { + Some(electra_epoch) if epoch >= electra_epoch => self.max_blobs_per_block_electra, + _ => self.max_blobs_per_block, + }, + } } - /// Return the value of `MAX_BLOBS_PER_BLOCK` appropriate for `fork`. - pub fn max_blobs_per_block_by_fork(&self, fork_name: ForkName) -> u64 { - if fork_name.fulu_enabled() { - self.max_blobs_per_block_fulu - } else if fork_name.electra_enabled() { - self.max_blobs_per_block_electra + // TODO(EIP-7892): remove this once we have fork-version changes on BPO forks + pub fn max_blobs_per_block_within_fork(&self, fork_name: ForkName) -> u64 { + if !fork_name.fulu_enabled() { + if fork_name.electra_enabled() { + self.max_blobs_per_block_electra + } else { + self.max_blobs_per_block + } } else { - self.max_blobs_per_block + // Find the max blobs per block in the fork schedule + // This logic will need to be more complex once there are forks beyond Fulu + let mut max_blobs_per_block = self.max_blobs_per_block_electra; + for entry in &self.blob_schedule { + if entry.max_blobs_per_block > max_blobs_per_block { + max_blobs_per_block = entry.max_blobs_per_block; + } + } + max_blobs_per_block } } @@ -1002,7 +1023,7 @@ impl ChainSpec { /* * Networking Fulu specific */ - max_blobs_per_block_fulu: default_max_blobs_per_block_fulu(), + blob_schedule: BlobSchedule::default(), /* * Application specific @@ -1336,7 +1357,7 @@ impl ChainSpec { /* * Networking Fulu specific */ - max_blobs_per_block_fulu: default_max_blobs_per_block_fulu(), + blob_schedule: BlobSchedule::default(), /* * Application specific @@ -1357,6 +1378,75 @@ impl Default for ChainSpec { } } +#[derive(arbitrary::Arbitrary, Serialize, Deserialize, Debug, PartialEq, Clone)] +#[serde(rename_all = "UPPERCASE")] +pub struct BPOFork { + epoch: Epoch, + #[serde(with = "serde_utils::quoted_u64")] + max_blobs_per_block: u64, +} + +// A wrapper around a vector of BPOFork to ensure that the vector is reverse +// sorted by epoch. +#[derive(arbitrary::Arbitrary, Serialize, Debug, PartialEq, Clone)] +pub struct BlobSchedule(Vec); + +impl<'de> Deserialize<'de> for BlobSchedule { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let vec = Vec::::deserialize(deserializer)?; + Ok(BlobSchedule::new(vec)) + } +} + +impl BlobSchedule { + pub fn new(mut vec: Vec) -> Self { + // reverse sort by epoch + vec.sort_by(|a, b| b.epoch.cmp(&a.epoch)); + Self(vec) + } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + pub fn max_blobs_for_epoch(&self, epoch: Epoch) -> Option { + self.0 + .iter() + .find(|entry| epoch >= entry.epoch) + .map(|entry| entry.max_blobs_per_block) + } + + pub const fn default() -> Self { + // TODO(EIP-7892): think about what the default should be + Self(vec![]) + } + + pub fn as_vec(&self) -> &Vec { + &self.0 + } +} + +impl<'a> IntoIterator for &'a BlobSchedule { + type Item = &'a BPOFork; + type IntoIter = std::slice::Iter<'a, BPOFork>; + + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } +} + +impl IntoIterator for BlobSchedule { + type Item = BPOFork; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + /// Exact implementation of the *config* object from the Ethereum spec (YAML/JSON). /// /// Fields relevant to hard forks after Altair should be optional so that we can continue @@ -1557,9 +1647,9 @@ pub struct Config { #[serde(default = "default_custody_requirement")] #[serde(with = "serde_utils::quoted_u64")] custody_requirement: u64, - #[serde(default = "default_max_blobs_per_block_fulu")] - #[serde(with = "serde_utils::quoted_u64")] - max_blobs_per_block_fulu: u64, + #[serde(default = "BlobSchedule::default")] + #[serde(skip_serializing_if = "BlobSchedule::is_empty")] + blob_schedule: BlobSchedule, } fn default_bellatrix_fork_version() -> [u8; 4] { @@ -1697,10 +1787,6 @@ const fn default_max_blobs_per_block_electra() -> u64 { 9 } -const fn default_max_blobs_per_block_fulu() -> u64 { - 12 -} - const fn default_attestation_propagation_slot_range() -> u64 { 32 } @@ -1937,7 +2023,7 @@ impl Config { data_column_sidecar_subnet_count: spec.data_column_sidecar_subnet_count, samples_per_slot: spec.samples_per_slot, custody_requirement: spec.custody_requirement, - max_blobs_per_block_fulu: spec.max_blobs_per_block_fulu, + blob_schedule: spec.blob_schedule.clone(), } } @@ -2016,7 +2102,7 @@ impl Config { data_column_sidecar_subnet_count, samples_per_slot, custody_requirement, - max_blobs_per_block_fulu, + ref blob_schedule, } = self; if preset_base != E::spec_name().to_string().as_str() { @@ -2100,7 +2186,7 @@ impl Config { data_column_sidecar_subnet_count, samples_per_slot, custody_requirement, - max_blobs_per_block_fulu, + blob_schedule: blob_schedule.clone(), ..chain_spec.clone() }) @@ -2287,6 +2373,140 @@ mod yaml_tests { assert_eq!(from, yamlconfig); } + #[test] + fn blob_schedule_max_blobs_per_block() { + let spec_contents = r#" + PRESET_BASE: 'mainnet' + MIN_GENESIS_ACTIVE_VALIDATOR_COUNT: 384 + MIN_GENESIS_TIME: 1748264340 + GENESIS_FORK_VERSION: 0x10355025 + GENESIS_DELAY: 60 + SECONDS_PER_SLOT: 12 + SECONDS_PER_ETH1_BLOCK: 12 + MIN_VALIDATOR_WITHDRAWABILITY_DELAY: 256 + SHARD_COMMITTEE_PERIOD: 256 + ETH1_FOLLOW_DISTANCE: 2048 + INACTIVITY_SCORE_BIAS: 4 + INACTIVITY_SCORE_RECOVERY_RATE: 16 + EJECTION_BALANCE: 16000000000 + MIN_PER_EPOCH_CHURN_LIMIT: 4 + CHURN_LIMIT_QUOTIENT: 65536 + MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 8 + PROPOSER_SCORE_BOOST: 40 + REORG_HEAD_WEIGHT_THRESHOLD: 20 + REORG_PARENT_WEIGHT_THRESHOLD: 160 + REORG_MAX_EPOCHS_SINCE_FINALIZATION: 2 + DEPOSIT_CHAIN_ID: 7042643276 + DEPOSIT_NETWORK_ID: 7042643276 + DEPOSIT_CONTRACT_ADDRESS: 0x00000000219ab540356cBB839Cbe05303d7705Fa + + ALTAIR_FORK_VERSION: 0x20355025 + ALTAIR_FORK_EPOCH: 0 + BELLATRIX_FORK_VERSION: 0x30355025 + BELLATRIX_FORK_EPOCH: 0 + CAPELLA_FORK_VERSION: 0x40355025 + CAPELLA_FORK_EPOCH: 0 + DENEB_FORK_VERSION: 0x50355025 + DENEB_FORK_EPOCH: 64 + ELECTRA_FORK_VERSION: 0x60355025 + ELECTRA_FORK_EPOCH: 128 + FULU_FORK_VERSION: 0x70355025 + FULU_FORK_EPOCH: 256 + BLOB_SCHEDULE: + - EPOCH: 512 + MAX_BLOBS_PER_BLOCK: 12 + - EPOCH: 768 + MAX_BLOBS_PER_BLOCK: 15 + - EPOCH: 1024 + MAX_BLOBS_PER_BLOCK: 18 + - EPOCH: 1280 + MAX_BLOBS_PER_BLOCK: 9 + - EPOCH: 1584 + MAX_BLOBS_PER_BLOCK: 20 + "#; + let config: Config = + serde_yaml::from_str(spec_contents).expect("error while deserializing"); + let spec = + ChainSpec::from_config::(&config).expect("error while creating spec"); + + // test out max_blobs_per_block(epoch) + assert_eq!( + spec.max_blobs_per_block(Epoch::new(64)), + default_max_blobs_per_block() + ); + assert_eq!( + spec.max_blobs_per_block(Epoch::new(127)), + default_max_blobs_per_block() + ); + assert_eq!( + spec.max_blobs_per_block(Epoch::new(128)), + default_max_blobs_per_block_electra() + ); + assert_eq!( + spec.max_blobs_per_block(Epoch::new(255)), + default_max_blobs_per_block_electra() + ); + assert_eq!( + spec.max_blobs_per_block(Epoch::new(256)), + default_max_blobs_per_block_electra() + ); + assert_eq!( + spec.max_blobs_per_block(Epoch::new(511)), + default_max_blobs_per_block_electra() + ); + assert_eq!(spec.max_blobs_per_block(Epoch::new(512)), 12); + assert_eq!(spec.max_blobs_per_block(Epoch::new(767)), 12); + assert_eq!(spec.max_blobs_per_block(Epoch::new(768)), 15); + assert_eq!(spec.max_blobs_per_block(Epoch::new(1023)), 15); + assert_eq!(spec.max_blobs_per_block(Epoch::new(1024)), 18); + assert_eq!(spec.max_blobs_per_block(Epoch::new(1279)), 18); + assert_eq!(spec.max_blobs_per_block(Epoch::new(1280)), 9); + assert_eq!(spec.max_blobs_per_block(Epoch::new(1583)), 9); + assert_eq!(spec.max_blobs_per_block(Epoch::new(1584)), 20); + assert_eq!( + spec.max_blobs_per_block(Epoch::new(18446744073709551615)), + 20 + ); + + // blob schedule is reverse sorted by epoch + assert_eq!( + config.blob_schedule.as_vec(), + &vec![ + BPOFork { + epoch: Epoch::new(1584), + max_blobs_per_block: 20 + }, + BPOFork { + epoch: Epoch::new(1280), + max_blobs_per_block: 9 + }, + BPOFork { + epoch: Epoch::new(1024), + max_blobs_per_block: 18 + }, + BPOFork { + epoch: Epoch::new(768), + max_blobs_per_block: 15 + }, + BPOFork { + epoch: Epoch::new(512), + max_blobs_per_block: 12 + }, + ] + ); + + // test max_blobs_per_block_within_fork + assert_eq!( + spec.max_blobs_per_block_within_fork(ForkName::Deneb), + default_max_blobs_per_block() + ); + assert_eq!( + spec.max_blobs_per_block_within_fork(ForkName::Electra), + default_max_blobs_per_block_electra() + ); + assert_eq!(spec.max_blobs_per_block_within_fork(ForkName::Fulu), 20); + } + #[test] fn apply_to_spec() { let mut spec = ChainSpec::minimal();