From 9fc502a3a6d69ebfce1a1430ec7b395cac5f02bd Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 9 Jul 2025 16:22:25 +1000 Subject: [PATCH 01/10] Clean up and add more code comments on BPO forks. --- beacon_node/beacon_chain/src/beacon_chain.rs | 9 ++++++ beacon_node/http_api/src/light_client.rs | 10 +++--- .../lighthouse_network/src/discovery/enr.rs | 32 ++++++++++--------- .../lighthouse_network/src/discovery/mod.rs | 12 +++++-- .../lighthouse_network/src/service/mod.rs | 10 ++---- beacon_node/network/src/service.rs | 3 +- consensus/types/src/chain_spec.rs | 28 ++++++---------- consensus/types/src/enr_fork_id.rs | 6 +++- consensus/types/src/fork_context.rs | 11 ++++--- lcli/src/generate_bootnode_enr.rs | 4 +-- 10 files changed, 65 insertions(+), 60 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index ed8cc37cc42..e0fc03de06d 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -6804,6 +6804,15 @@ impl BeaconChain { .enr_fork_id::(slot, self.genesis_validators_root) } + /// Returns the fork_digest corresponding to an epoch. + /// See [`ChainSpec::compute_fork_digest`] + pub fn compute_fork_digest(&self, epoch: Epoch) -> [u8; 4] { + self.spec + .compute_fork_digest(self.genesis_validators_root, epoch) + } + + /// Calculates the `Duration` to the next fork digest (this could be either a regular or BPO + /// hard fork) if it exists and returns it with its corresponding `Epoch`. pub fn duration_to_next_digest(&self) -> Option<(Epoch, Duration)> { // If we are unable to read the slot clock we assume that it is prior to genesis and // therefore use the genesis slot. diff --git a/beacon_node/http_api/src/light_client.rs b/beacon_node/http_api/src/light_client.rs index a51c4acc719..f9559d738ea 100644 --- a/beacon_node/http_api/src/light_client.rs +++ b/beacon_node/http_api/src/light_client.rs @@ -150,12 +150,10 @@ fn map_light_client_update_to_ssz_chunk( chain: &BeaconChain, light_client_update: &LightClientUpdate, ) -> LightClientUpdateResponseChunk { - let fork_digest = chain.spec.compute_fork_digest( - chain.genesis_validators_root, - light_client_update - .attested_header_slot() - .epoch(T::EthSpec::slots_per_epoch()), - ); + let epoch = light_client_update + .attested_header_slot() + .epoch(T::EthSpec::slots_per_epoch()); + let fork_digest = chain.compute_fork_digest(epoch); let payload = light_client_update.as_ssz_bytes(); let response_chunk_len = fork_digest.len() + payload.len(); diff --git a/beacon_node/lighthouse_network/src/discovery/enr.rs b/beacon_node/lighthouse_network/src/discovery/enr.rs index 017db1fdc73..afa8768f6dd 100644 --- a/beacon_node/lighthouse_network/src/discovery/enr.rs +++ b/beacon_node/lighthouse_network/src/discovery/enr.rs @@ -3,7 +3,9 @@ pub use discv5::enr::CombinedKey; use super::enr_ext::CombinedKeyExt; +use super::enr_ext::{EnrExt, QUIC6_ENR_KEY, QUIC_ENR_KEY}; use super::ENR_FILENAME; +use crate::config::Config; use crate::types::{Enr, EnrAttestationBitfield, EnrSyncCommitteeBitfield}; use crate::NetworkConfig; use alloy_rlp::bytes::Bytes; @@ -18,8 +20,6 @@ use std::str::FromStr; use tracing::{debug, warn}; use types::{ChainSpec, EnrForkId, EthSpec}; -use super::enr_ext::{EnrExt, QUIC6_ENR_KEY, QUIC_ENR_KEY}; - /// The ENR field specifying the fork id. pub const ETH2_ENR_KEY: &str = "eth2"; /// The ENR field specifying the next fork digest. @@ -87,8 +87,7 @@ impl Eth2Enr for Enr { } fn next_fork_digest(&self) -> Result<[u8; 4], &'static str> { - self - .get_decodable::<[u8; 4]>(NEXT_FORK_DIGEST_ENR_KEY) + self.get_decodable::<[u8; 4]>(NEXT_FORK_DIGEST_ENR_KEY) .ok_or("ENR next fork digest non-existent")? .map_err(|_| "Could not decode the ENR next fork digest") } @@ -161,14 +160,14 @@ pub fn build_or_load_enr( local_key: Keypair, config: &NetworkConfig, enr_fork_id: &EnrForkId, + next_fork_digest: [u8; 4], spec: &ChainSpec, - nfd: [u8; 4], ) -> Result { // Build the local ENR. // Note: Discovery should update the ENR record's IP to the external IP as seen by the // majority of our peers, if the CLI doesn't expressly forbid it. let enr_key = CombinedKey::from_libp2p(local_key)?; - let mut local_enr = build_enr::(&enr_key, config, enr_fork_id, spec, nfd)?; + let mut local_enr = build_enr::(&enr_key, config, enr_fork_id, next_fork_digest, spec)?; use_or_load_enr(&enr_key, &mut local_enr, config)?; Ok(local_enr) @@ -179,8 +178,8 @@ pub fn build_enr( enr_key: &CombinedKey, config: &NetworkConfig, enr_fork_id: &EnrForkId, + next_fork_digest: [u8; 4], spec: &ChainSpec, - nfd: [u8; 4], ) -> Result { let mut builder = discv5::enr::Enr::builder(); let (maybe_ipv4_address, maybe_ipv6_address) = &config.enr_address; @@ -271,7 +270,7 @@ pub fn build_enr( &bitfield.as_ssz_bytes().into(), ); - // only set `cgc` if PeerDAS fork epoch has been scheduled + // only set `cgc` and `nfd` if PeerDAS fork (Fulu) epoch has been scheduled if spec.is_peer_das_scheduled() { let custody_group_count = if let Some(false_cgc) = config.advertise_false_custody_group_count { @@ -282,11 +281,7 @@ pub fn build_enr( spec.custody_requirement }; builder.add_value(PEERDAS_CUSTODY_GROUP_COUNT_ENR_KEY, &custody_group_count); - } - - // only set `nfd` if peer das is scheduled - if spec.is_peer_das_scheduled() { - builder.add_value(NEXT_FORK_DIGEST_ENR_KEY, &nfd); + builder.add_value(NEXT_FORK_DIGEST_ENR_KEY, &next_fork_digest); } builder @@ -359,6 +354,7 @@ mod test { use types::{Epoch, MainnetEthSpec}; type E = MainnetEthSpec; + const TEST_NFD: [u8; 4] = [0x01, 0x02, 0x03, 0x04]; fn make_fulu_spec() -> ChainSpec { let mut spec = E::default_spec(); @@ -370,11 +366,17 @@ mod test { let keypair = libp2p::identity::secp256k1::Keypair::generate(); let enr_key = CombinedKey::from_secp256k1(&keypair); let enr_fork_id = EnrForkId::default(); - let nfd = [0; 4]; // placeholder - let enr = build_enr::(&enr_key, &config, &enr_fork_id, spec, nfd).unwrap(); + let enr = build_enr::(&enr_key, &config, &enr_fork_id, TEST_NFD, spec).unwrap(); (enr, enr_key) } + #[test] + fn test_nfd_enr_encoding() { + let spec = make_fulu_spec(); + let enr = build_enr_with_config(NetworkConfig::default(), &spec).0; + assert_eq!(enr.next_fork_digest().unwrap(), TEST_NFD); + } + #[test] fn custody_group_count_default() { let config = NetworkConfig { diff --git a/beacon_node/lighthouse_network/src/discovery/mod.rs b/beacon_node/lighthouse_network/src/discovery/mod.rs index f3351719e7f..df866dfc646 100644 --- a/beacon_node/lighthouse_network/src/discovery/mod.rs +++ b/beacon_node/lighthouse_network/src/discovery/mod.rs @@ -1230,9 +1230,15 @@ mod tests { config.set_listening_addr(crate::ListenAddress::unused_v4_ports()); let config = Arc::new(config); let enr_key: CombinedKey = CombinedKey::from_secp256k1(&keypair); - let nfd = [0; 4]; // placeholder - let enr: Enr = - build_enr::(&enr_key, &config, &EnrForkId::default(), &spec, nfd).unwrap(); + let next_fork_digest = [0; 4]; + let enr: Enr = build_enr::( + &enr_key, + &config, + &EnrForkId::default(), + next_fork_digest, + &spec, + ) + .unwrap(); let globals = NetworkGlobals::new( enr, MetaData::V2(MetaDataV2 { diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index 033bc1346b6..17965fd9ee7 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -198,8 +198,8 @@ impl Network { local_keypair.clone(), &config, &ctx.enr_fork_id, - &ctx.chain_spec, ctx.fork_context.next_fork_digest(), + &ctx.chain_spec, )?; // Construct the metadata @@ -1361,15 +1361,9 @@ impl Network { self.enr_fork_id = enr_fork_id; } - #[instrument(parent = None, - level = "trace", - fields(service = "libp2p"), - name = "libp2p", - skip_all - )] pub fn update_nfd(&mut self, nfd: [u8; 4]) { if let Err(e) = self.discovery_mut().update_enr_nfd(nfd) { - warn!(error = %e, "Could not update nfd in ENR"); + crit!(error = e, "Could not update nfd in ENR"); } } diff --git a/beacon_node/network/src/service.rs b/beacon_node/network/src/service.rs index 81b1b64d429..c3bf98917f3 100644 --- a/beacon_node/network/src/service.rs +++ b/beacon_node/network/src/service.rs @@ -452,7 +452,7 @@ impl NetworkService { Some(_) = &mut self.next_topic_subscriptions => { if let Some((epoch, _)) = self.beacon_chain.duration_to_next_digest() { let fork_name = self.beacon_chain.spec.fork_name_at_epoch(epoch); - let fork_digest = self.beacon_chain.spec.compute_fork_digest(self.beacon_chain.genesis_validators_root, epoch); + let fork_digest = self.beacon_chain.compute_fork_digest(epoch); info!("Subscribing to new fork topics"); self.libp2p.subscribe_new_fork_topics(fork_name, fork_digest); self.next_topic_subscriptions = Box::pin(None.into()); @@ -829,7 +829,6 @@ impl NetworkService { let fork_context = &self.fork_context; if let Some(new_fork_name) = fork_context.from_context_bytes(new_fork_digest) { if fork_context.current_fork() == *new_fork_name { - // BPO FORK info!( epoch = ?current_epoch, "BPO Fork Triggered" diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 59d428315d0..915935b2e82 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -441,8 +441,13 @@ impl ChainSpec { .is_some_and(|fulu_fork_epoch| block_epoch >= fulu_fork_epoch) } - /// Returns true if `FULU_FORK_EPOCH` is set and is not set to `FAR_FUTURE_EPOCH`. + /// Returns true if PeerDAS is scheduled. Alias for [`Self::is_fulu_scheduled`] pub fn is_peer_das_scheduled(&self) -> bool { + self.is_fulu_scheduled() + } + + /// Returns true if `FULU_FORK_EPOCH` is set and is not set to `FAR_FUTURE_EPOCH`. + pub fn is_fulu_scheduled(&self) -> bool { self.fulu_fork_epoch .is_some_and(|fulu_fork_epoch| fulu_fork_epoch != self.far_future_epoch) } @@ -593,7 +598,7 @@ impl ChainSpec { .filter_map(|(_, epoch)| epoch) .collect::>(); - if self.fulu_fork_epoch.is_some() { + if self.is_fulu_scheduled() { for blob_parameters in &self.blob_schedule { relevant_epochs.insert(blob_parameters.epoch); } @@ -717,7 +722,8 @@ impl ChainSpec { } } - pub fn get_blob_parameters(&self, epoch: Epoch) -> Option { + /// Return the blob parameters at a given epoch. + fn get_blob_parameters(&self, epoch: Epoch) -> Option { match self.fulu_fork_epoch { Some(fulu_epoch) if epoch >= fulu_epoch => self .blob_schedule @@ -730,19 +736,7 @@ impl ChainSpec { max_blobs_per_block: self.max_blobs_per_block_electra, }) }), - _ => match self.electra_fork_epoch { - Some(electra_epoch) if epoch >= electra_epoch => Some(BlobParameters { - epoch: electra_epoch, - max_blobs_per_block: self.max_blobs_per_block_electra, - }), - _ => match self.deneb_fork_epoch { - Some(deneb_epoch) if epoch >= deneb_epoch => Some(BlobParameters { - epoch: deneb_epoch, - max_blobs_per_block: self.max_blobs_per_block, - }), - _ => None, - }, - }, + _ => None, } } @@ -1522,7 +1516,6 @@ impl BlobSchedule { } pub const fn default() -> Self { - // TODO(EIP-7892): think about what the default should be Self(vec![]) } @@ -1769,7 +1762,6 @@ fn default_bellatrix_fork_version() -> [u8; 4] { } fn default_capella_fork_version() -> [u8; 4] { - // TODO: determine if the bellatrix example should be copied like this [0xff, 0xff, 0xff, 0xff] } diff --git a/consensus/types/src/enr_fork_id.rs b/consensus/types/src/enr_fork_id.rs index 3ae7c39cfe9..5d00e927833 100644 --- a/consensus/types/src/enr_fork_id.rs +++ b/consensus/types/src/enr_fork_id.rs @@ -1,5 +1,5 @@ use crate::test_utils::TestRandom; -use crate::Epoch; +use crate::{ChainSpec, Epoch}; use serde::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; @@ -24,10 +24,14 @@ use tree_hash_derive::TreeHash; TestRandom, )] pub struct EnrForkId { + /// Fork digest of the current fork computed from [`ChainSpec::compute_fork_digest`]. #[serde(with = "serde_utils::bytes_4_hex")] pub fork_digest: [u8; 4], + /// `next_fork_version` is the fork version corresponding to the next planned fork at a future + /// epoch. The fork version will only change for regular forks, not BPO forks. #[serde(with = "serde_utils::bytes_4_hex")] pub next_fork_version: [u8; 4], + /// `next_fork_epoch` is the epoch at which the next fork (whether a regular fork or a BPO fork) is planned pub next_fork_epoch: Epoch, } diff --git a/consensus/types/src/fork_context.rs b/consensus/types/src/fork_context.rs index 86a93323d95..8e79b9a8aef 100644 --- a/consensus/types/src/fork_context.rs +++ b/consensus/types/src/fork_context.rs @@ -1,11 +1,13 @@ use parking_lot::RwLock; use crate::{ChainSpec, Epoch, EthSpec, ForkName, Hash256, Slot}; -use std::collections::{ HashMap, HashSet}; +use std::collections::{HashMap, HashSet}; /// Provides fork specific info like the current fork name and the fork digests corresponding to every valid fork. #[derive(Debug)] pub struct ForkContext { + /// Activation epoch of the current hard fork. This can be either a named fork (`ForkName`) or + /// an unnamed blob parameter only fork (BPO). digest_epoch: RwLock, enabled_forks: HashSet, genesis_validators_root: Hash256, @@ -51,7 +53,7 @@ impl ForkContext { .filter(|&&epoch| epoch <= current_epoch) .max() .cloned() - .expect("should match atleast genesis epoch"), + .expect("should match at least genesis epoch"), ); Self { @@ -73,7 +75,7 @@ impl ForkContext { self.spec.fork_name_at_epoch(self.digest_epoch()) } - /// Returns the current digest epoch + /// Returns the current digest epoch. pub fn digest_epoch(&self) -> Epoch { *self.digest_epoch.read() } @@ -105,7 +107,8 @@ impl ForkContext { self.digest_to_fork.get(&context) } - // TODO: we *may* delete this entire object and just use the spec + /// Returns the context bytes/fork_digest corresponding to an epoch. + /// See [`ChainSpec::compute_fork_digest`] pub fn context_bytes(&self, epoch: Epoch) -> [u8; 4] { self.spec .compute_fork_digest(self.genesis_validators_root, epoch) diff --git a/lcli/src/generate_bootnode_enr.rs b/lcli/src/generate_bootnode_enr.rs index 7334f552576..75969d676fa 100644 --- a/lcli/src/generate_bootnode_enr.rs +++ b/lcli/src/generate_bootnode_enr.rs @@ -15,7 +15,6 @@ pub fn run(matches: &ArgMatches, spec: &ChainSpec) -> Result<(), Str let udp_port: NonZeroU16 = clap_utils::parse_required(matches, "udp-port")?; let tcp_port: NonZeroU16 = clap_utils::parse_required(matches, "tcp-port")?; let output_dir: PathBuf = clap_utils::parse_required(matches, "output-dir")?; - // FIXME: why is this being read from.. somewhere rather than just using the spec? let genesis_fork_version: [u8; 4] = clap_utils::parse_ssz_required(matches, "genesis-fork-version")?; @@ -38,8 +37,7 @@ pub fn run(matches: &ArgMatches, spec: &ChainSpec) -> Result<(), Str next_fork_version: genesis_fork_version, next_fork_epoch: Epoch::max_value(), // FAR_FUTURE_EPOCH }; - // FIXME: need the next fork digest - let enr = build_enr::(&enr_key, &config, &enr_fork_id, spec, [0; 4]) + let enr = build_enr::(&enr_key, &config, &enr_fork_id, [0; 4], spec) .map_err(|e| format!("Unable to create ENR: {:?}", e))?; fs::create_dir_all(&output_dir).map_err(|e| format!("Unable to create output-dir: {:?}", e))?; From c84b1d37490282dc1e2b946396ce84ca1475c2ca Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 9 Jul 2025 16:33:13 +1000 Subject: [PATCH 02/10] Fix tests. --- .../lighthouse_network/src/discovery/enr.rs | 1 - beacon_node/lighthouse_network/src/rpc/codec.rs | 15 +++++++++------ beacon_node/network/src/service/tests.rs | 9 ++++----- consensus/types/src/enr_fork_id.rs | 2 +- lcli/src/generate_bootnode_enr.rs | 5 +++-- 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/beacon_node/lighthouse_network/src/discovery/enr.rs b/beacon_node/lighthouse_network/src/discovery/enr.rs index afa8768f6dd..4c055604979 100644 --- a/beacon_node/lighthouse_network/src/discovery/enr.rs +++ b/beacon_node/lighthouse_network/src/discovery/enr.rs @@ -5,7 +5,6 @@ pub use discv5::enr::CombinedKey; use super::enr_ext::CombinedKeyExt; use super::enr_ext::{EnrExt, QUIC6_ENR_KEY, QUIC_ENR_KEY}; use super::ENR_FILENAME; -use crate::config::Config; use crate::types::{Enr, EnrAttestationBitfield, EnrSyncCommitteeBitfield}; use crate::NetworkConfig; use alloy_rlp::bytes::Bytes; diff --git a/beacon_node/lighthouse_network/src/rpc/codec.rs b/beacon_node/lighthouse_network/src/rpc/codec.rs index 553ed9aff97..e61e78dc5a5 100644 --- a/beacon_node/lighthouse_network/src/rpc/codec.rs +++ b/beacon_node/lighthouse_network/src/rpc/codec.rs @@ -1135,7 +1135,7 @@ mod tests { let mut dst = BytesMut::new(); // Add context bytes if required - dst.extend_from_slice(&fork_context.to_context_bytes(fork_name).unwrap()); + dst.extend_from_slice(&fork_context.context_bytes(fork_context.digest_epoch())); let mut uvi_codec: Uvi = Uvi::default(); @@ -1792,8 +1792,8 @@ mod tests { .unwrap(); let mut wrong_fork_bytes = BytesMut::new(); - wrong_fork_bytes - .extend_from_slice(&fork_context.to_context_bytes(ForkName::Altair).unwrap()); + let altair_epoch = chain_spec.altair_fork_epoch.unwrap(); + wrong_fork_bytes.extend_from_slice(&fork_context.context_bytes(altair_epoch)); wrong_fork_bytes.extend_from_slice(&encoded_bytes.split_off(4)); assert!(matches!( @@ -1817,7 +1817,9 @@ mod tests { .unwrap(); let mut wrong_fork_bytes = BytesMut::new(); - wrong_fork_bytes.extend_from_slice(&fork_context.to_context_bytes(ForkName::Base).unwrap()); + wrong_fork_bytes.extend_from_slice( + &fork_context.context_bytes(chain_spec.genesis_slot.epoch(Spec::slots_per_epoch())), + ); wrong_fork_bytes.extend_from_slice(&encoded_bytes.split_off(4)); assert!(matches!( @@ -1833,7 +1835,7 @@ mod tests { // Adding context bytes to Protocols that don't require it should return an error let mut encoded_bytes = BytesMut::new(); - encoded_bytes.extend_from_slice(&fork_context.to_context_bytes(ForkName::Altair).unwrap()); + encoded_bytes.extend_from_slice(&fork_context.context_bytes(altair_epoch)); encoded_bytes.extend_from_slice( &encode_response( SupportedProtocol::MetaDataV2, @@ -2034,7 +2036,8 @@ mod tests { let mut dst = BytesMut::with_capacity(1024); // Insert context bytes - dst.extend_from_slice(&fork_context.to_context_bytes(ForkName::Altair).unwrap()); + let altair_epoch = fork_context.spec.altair_fork_epoch.unwrap(); + dst.extend_from_slice(&fork_context.context_bytes(altair_epoch)); // Insert length-prefix uvi_codec diff --git a/beacon_node/network/src/service/tests.rs b/beacon_node/network/src/service/tests.rs index db342117473..a8f68384a02 100644 --- a/beacon_node/network/src/service/tests.rs +++ b/beacon_node/network/src/service/tests.rs @@ -11,7 +11,7 @@ use lighthouse_network::{Enr, GossipTopic}; use std::str::FromStr; use std::sync::Arc; use tokio::runtime::Runtime; -use types::{Epoch, EthSpec, ForkName, MinimalEthSpec, SubnetId}; +use types::{Epoch, EthSpec, MinimalEthSpec, SubnetId}; impl NetworkService { fn get_topic_params(&self, topic: GossipTopic) -> Option<&gossipsub::TopicScoreParams> { @@ -106,8 +106,8 @@ fn test_removing_topic_weight_on_old_topics() { .mock_execution_layer() .build() .chain; - let (next_fork_name, _) = beacon_chain.duration_to_next_fork().expect("next fork"); - assert_eq!(next_fork_name, ForkName::Capella); + let (next_fork_epoch, _) = beacon_chain.duration_to_next_digest().expect("next fork"); + assert_eq!(Some(next_fork_epoch), spec.capella_fork_epoch); // Build network service. let (mut network_service, network_globals, _network_senders) = runtime.block_on(async { @@ -189,9 +189,8 @@ fn test_removing_topic_weight_on_old_topics() { beacon_chain.slot_clock.advance_slot(); } - // Run `NetworkService::update_next_fork()`. runtime.block_on(async { - network_service.update_next_fork(); + network_service.update_next_fork_digest(); }); // Check that topic_weight on the old topics has been zeroed. diff --git a/consensus/types/src/enr_fork_id.rs b/consensus/types/src/enr_fork_id.rs index 5d00e927833..e3742cb96c1 100644 --- a/consensus/types/src/enr_fork_id.rs +++ b/consensus/types/src/enr_fork_id.rs @@ -1,5 +1,5 @@ use crate::test_utils::TestRandom; -use crate::{ChainSpec, Epoch}; +use crate::Epoch; use serde::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; diff --git a/lcli/src/generate_bootnode_enr.rs b/lcli/src/generate_bootnode_enr.rs index 75969d676fa..98ef0b96d43 100644 --- a/lcli/src/generate_bootnode_enr.rs +++ b/lcli/src/generate_bootnode_enr.rs @@ -32,12 +32,13 @@ pub fn run(matches: &ArgMatches, spec: &ChainSpec) -> Result<(), Str let secp256k1_keypair = secp256k1::Keypair::generate(); let enr_key = CombinedKey::from_secp256k1(&secp256k1_keypair); + let genesis_fork_digest = spec.compute_fork_digest(Hash256::zero(), Epoch::new(0)); let enr_fork_id = EnrForkId { - fork_digest: spec.compute_fork_digest(Hash256::zero(), Epoch::new(0)), + fork_digest: genesis_fork_digest, next_fork_version: genesis_fork_version, next_fork_epoch: Epoch::max_value(), // FAR_FUTURE_EPOCH }; - let enr = build_enr::(&enr_key, &config, &enr_fork_id, [0; 4], spec) + let enr = build_enr::(&enr_key, &config, &enr_fork_id, genesis_fork_digest, spec) .map_err(|e| format!("Unable to create ENR: {:?}", e))?; fs::create_dir_all(&output_dir).map_err(|e| format!("Unable to create output-dir: {:?}", e))?; From 87a9f7d17bfbd0d975fca910b24232074d7d1e85 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 9 Jul 2025 17:04:34 +1000 Subject: [PATCH 03/10] Fix tests. --- consensus/types/src/fork_name.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/consensus/types/src/fork_name.rs b/consensus/types/src/fork_name.rs index e92db494851..4fc26ccffa8 100644 --- a/consensus/types/src/fork_name.rs +++ b/consensus/types/src/fork_name.rs @@ -36,8 +36,6 @@ impl ForkName { pub fn list_all_fork_epochs(spec: &ChainSpec) -> Vec<(ForkName, Option)> { ForkName::list_all() .into_iter() - // Skip Base - .skip(1) .map(|fork| (fork, spec.fork_epoch(fork))) .collect() } From 86cebb6c5123eb766e84ff8d810a49f849a7ac15 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 9 Jul 2025 17:57:18 +1000 Subject: [PATCH 04/10] Fix moar tests. --- .../lighthouse_network/src/rpc/codec.rs | 160 +++++++++++++----- 1 file changed, 114 insertions(+), 46 deletions(-) diff --git a/beacon_node/lighthouse_network/src/rpc/codec.rs b/beacon_node/lighthouse_network/src/rpc/codec.rs index e61e78dc5a5..aa54db5b4af 100644 --- a/beacon_node/lighthouse_network/src/rpc/codec.rs +++ b/beacon_node/lighthouse_network/src/rpc/codec.rs @@ -945,34 +945,62 @@ mod tests { SignedBeaconBlock::from_block(empty_block, Signature::empty()) } - fn altair_block() -> SignedBeaconBlock { - let full_block = + fn altair_block(spec: &ChainSpec) -> SignedBeaconBlock { + // The context bytes are now derived from the block epoch, so we need to have the slot set + // here. + let mut full_block = BeaconBlock::Altair(BeaconBlockAltair::::full(&Spec::default_spec())); + *full_block.slot_mut() = spec + .altair_fork_epoch + .expect("altair fork epoch must be set") + .start_slot(Spec::slots_per_epoch()); SignedBeaconBlock::from_block(full_block, Signature::empty()) } - fn empty_blob_sidecar() -> Arc> { - Arc::new(BlobSidecar::empty()) + fn empty_blob_sidecar(spec: &ChainSpec) -> Arc> { + // The context bytes are now derived from the block epoch, so we need to have the slot set + // here. + let mut blob_sidecar = BlobSidecar::::empty(); + blob_sidecar.signed_block_header.message.slot = spec + .deneb_fork_epoch + .expect("deneb fork epoch must be set") + .start_slot(Spec::slots_per_epoch()); + Arc::new(blob_sidecar) } - fn empty_data_column_sidecar() -> Arc> { - Arc::new(DataColumnSidecar { + fn empty_data_column_sidecar(spec: &ChainSpec) -> Arc> { + // The context bytes are now derived from the block epoch, so we need to have the slot set + // here. + let data_column_sidecar = DataColumnSidecar { index: 0, column: VariableList::new(vec![Cell::::default()]).unwrap(), kzg_commitments: VariableList::new(vec![KzgCommitment::empty_for_testing()]).unwrap(), kzg_proofs: VariableList::new(vec![KzgProof::empty()]).unwrap(), signed_block_header: SignedBeaconBlockHeader { - message: BeaconBlockHeader::empty(), + message: BeaconBlockHeader { + slot: spec + .fulu_fork_epoch + .expect("fulu fork epoch must be set") + .start_slot(Spec::slots_per_epoch()), + ..BeaconBlockHeader::empty() + }, signature: Signature::empty(), }, kzg_commitments_inclusion_proof: Default::default(), - }) + }; + Arc::new(data_column_sidecar) } /// Bellatrix block with length < max_rpc_size. fn bellatrix_block_small(spec: &ChainSpec) -> SignedBeaconBlock { + // The context bytes are now derived from the block epoch, so we need to have the slot set + // here. let mut block: BeaconBlockBellatrix<_, FullPayload> = - BeaconBlockBellatrix::empty(&Spec::default_spec()); + BeaconBlockBellatrix::empty(spec); + block.slot = spec + .bellatrix_fork_epoch + .expect("Bellatrix epoch must be set") + .start_slot(Spec::slots_per_epoch()); let tx = VariableList::from(vec![0; 1024]); let txs = VariableList::from(std::iter::repeat_n(tx, 5000).collect::>()); @@ -988,8 +1016,14 @@ mod tests { /// The max limit for a Bellatrix block is in the order of ~16GiB which wouldn't fit in memory. /// Hence, we generate a Bellatrix block just greater than `MAX_RPC_SIZE` to test rejection on the rpc layer. fn bellatrix_block_large(spec: &ChainSpec) -> SignedBeaconBlock { + // The context bytes are now derived from the block epoch, so we need to have the slot set + // here. let mut block: BeaconBlockBellatrix<_, FullPayload> = - BeaconBlockBellatrix::empty(&Spec::default_spec()); + BeaconBlockBellatrix::empty(spec); + block.slot = spec + .bellatrix_fork_epoch + .expect("Bellatrix epoch must be set") + .start_slot(Spec::slots_per_epoch()); let tx = VariableList::from(vec![0; 1024]); let txs = VariableList::from(std::iter::repeat_n(tx, 100000).collect::>()); @@ -1255,7 +1289,9 @@ mod tests { // Test RPCResponse encoding/decoding for V1 messages #[test] fn test_encode_then_decode_v1() { - let chain_spec = Spec::default_spec(); + let mut chain_spec = Spec::default_spec(); + // Set a fulu fork epoch so we can encode / decode data columns + chain_spec.fulu_fork_epoch = Some(Epoch::new(1401280)); assert_eq!( encode_then_decode_response( @@ -1307,7 +1343,7 @@ mod tests { encode_then_decode_response( SupportedProtocol::BlocksByRangeV1, RpcResponse::Success(RpcSuccessResponse::BlocksByRange(Arc::new( - altair_block() + altair_block(&chain_spec) ))), ForkName::Altair, &chain_spec, @@ -1336,9 +1372,9 @@ mod tests { matches!( encode_then_decode_response( SupportedProtocol::BlocksByRootV1, - RpcResponse::Success(RpcSuccessResponse::BlocksByRoot( - Arc::new(altair_block()) - )), + RpcResponse::Success(RpcSuccessResponse::BlocksByRoot(Arc::new(altair_block( + &chain_spec + )))), ForkName::Altair, &chain_spec, ) @@ -1383,74 +1419,98 @@ mod tests { assert_eq!( encode_then_decode_response( SupportedProtocol::BlobsByRangeV1, - RpcResponse::Success(RpcSuccessResponse::BlobsByRange(empty_blob_sidecar())), + RpcResponse::Success(RpcSuccessResponse::BlobsByRange(empty_blob_sidecar( + &chain_spec + ))), ForkName::Deneb, &chain_spec ), - Ok(Some(RpcSuccessResponse::BlobsByRange(empty_blob_sidecar()))), + Ok(Some(RpcSuccessResponse::BlobsByRange(empty_blob_sidecar( + &chain_spec + )))), ); assert_eq!( encode_then_decode_response( SupportedProtocol::BlobsByRangeV1, - RpcResponse::Success(RpcSuccessResponse::BlobsByRange(empty_blob_sidecar())), + RpcResponse::Success(RpcSuccessResponse::BlobsByRange(empty_blob_sidecar( + &chain_spec + ))), ForkName::Electra, &chain_spec ), - Ok(Some(RpcSuccessResponse::BlobsByRange(empty_blob_sidecar()))), + Ok(Some(RpcSuccessResponse::BlobsByRange(empty_blob_sidecar( + &chain_spec + )))), ); assert_eq!( encode_then_decode_response( SupportedProtocol::BlobsByRangeV1, - RpcResponse::Success(RpcSuccessResponse::BlobsByRange(empty_blob_sidecar())), + RpcResponse::Success(RpcSuccessResponse::BlobsByRange(empty_blob_sidecar( + &chain_spec + ))), ForkName::Fulu, &chain_spec ), - Ok(Some(RpcSuccessResponse::BlobsByRange(empty_blob_sidecar()))), + Ok(Some(RpcSuccessResponse::BlobsByRange(empty_blob_sidecar( + &chain_spec + )))), ); assert_eq!( encode_then_decode_response( SupportedProtocol::BlobsByRootV1, - RpcResponse::Success(RpcSuccessResponse::BlobsByRoot(empty_blob_sidecar())), + RpcResponse::Success(RpcSuccessResponse::BlobsByRoot(empty_blob_sidecar( + &chain_spec + ))), ForkName::Deneb, &chain_spec ), - Ok(Some(RpcSuccessResponse::BlobsByRoot(empty_blob_sidecar()))), + Ok(Some(RpcSuccessResponse::BlobsByRoot(empty_blob_sidecar( + &chain_spec + )))), ); assert_eq!( encode_then_decode_response( SupportedProtocol::BlobsByRootV1, - RpcResponse::Success(RpcSuccessResponse::BlobsByRoot(empty_blob_sidecar())), + RpcResponse::Success(RpcSuccessResponse::BlobsByRoot(empty_blob_sidecar( + &chain_spec + ))), ForkName::Electra, &chain_spec ), - Ok(Some(RpcSuccessResponse::BlobsByRoot(empty_blob_sidecar()))), + Ok(Some(RpcSuccessResponse::BlobsByRoot(empty_blob_sidecar( + &chain_spec + )))), ); assert_eq!( encode_then_decode_response( SupportedProtocol::BlobsByRootV1, - RpcResponse::Success(RpcSuccessResponse::BlobsByRoot(empty_blob_sidecar())), + RpcResponse::Success(RpcSuccessResponse::BlobsByRoot(empty_blob_sidecar( + &chain_spec + ))), ForkName::Fulu, &chain_spec ), - Ok(Some(RpcSuccessResponse::BlobsByRoot(empty_blob_sidecar()))), + Ok(Some(RpcSuccessResponse::BlobsByRoot(empty_blob_sidecar( + &chain_spec + )))), ); assert_eq!( encode_then_decode_response( SupportedProtocol::DataColumnsByRangeV1, RpcResponse::Success(RpcSuccessResponse::DataColumnsByRange( - empty_data_column_sidecar() + empty_data_column_sidecar(&chain_spec) )), ForkName::Deneb, &chain_spec ), Ok(Some(RpcSuccessResponse::DataColumnsByRange( - empty_data_column_sidecar() + empty_data_column_sidecar(&chain_spec) ))), ); @@ -1458,13 +1518,13 @@ mod tests { encode_then_decode_response( SupportedProtocol::DataColumnsByRangeV1, RpcResponse::Success(RpcSuccessResponse::DataColumnsByRange( - empty_data_column_sidecar() + empty_data_column_sidecar(&chain_spec) )), ForkName::Electra, &chain_spec ), Ok(Some(RpcSuccessResponse::DataColumnsByRange( - empty_data_column_sidecar() + empty_data_column_sidecar(&chain_spec) ))), ); @@ -1472,13 +1532,13 @@ mod tests { encode_then_decode_response( SupportedProtocol::DataColumnsByRangeV1, RpcResponse::Success(RpcSuccessResponse::DataColumnsByRange( - empty_data_column_sidecar() + empty_data_column_sidecar(&chain_spec) )), ForkName::Fulu, &chain_spec ), Ok(Some(RpcSuccessResponse::DataColumnsByRange( - empty_data_column_sidecar() + empty_data_column_sidecar(&chain_spec) ))), ); @@ -1486,13 +1546,13 @@ mod tests { encode_then_decode_response( SupportedProtocol::DataColumnsByRootV1, RpcResponse::Success(RpcSuccessResponse::DataColumnsByRoot( - empty_data_column_sidecar() + empty_data_column_sidecar(&chain_spec) )), ForkName::Deneb, &chain_spec ), Ok(Some(RpcSuccessResponse::DataColumnsByRoot( - empty_data_column_sidecar() + empty_data_column_sidecar(&chain_spec) ))), ); @@ -1500,13 +1560,13 @@ mod tests { encode_then_decode_response( SupportedProtocol::DataColumnsByRootV1, RpcResponse::Success(RpcSuccessResponse::DataColumnsByRoot( - empty_data_column_sidecar() + empty_data_column_sidecar(&chain_spec) )), ForkName::Electra, &chain_spec ), Ok(Some(RpcSuccessResponse::DataColumnsByRoot( - empty_data_column_sidecar() + empty_data_column_sidecar(&chain_spec) ))), ); @@ -1514,13 +1574,13 @@ mod tests { encode_then_decode_response( SupportedProtocol::DataColumnsByRootV1, RpcResponse::Success(RpcSuccessResponse::DataColumnsByRoot( - empty_data_column_sidecar() + empty_data_column_sidecar(&chain_spec) )), ForkName::Fulu, &chain_spec ), Ok(Some(RpcSuccessResponse::DataColumnsByRoot( - empty_data_column_sidecar() + empty_data_column_sidecar(&chain_spec) ))), ); } @@ -1528,7 +1588,9 @@ mod tests { // Test RPCResponse encoding/decoding for V1 messages #[test] fn test_encode_then_decode_v2() { - let chain_spec = Spec::default_spec(); + let mut chain_spec = Spec::default_spec(); + // Set a fulu fork epoch so we can encode / decode data columns + chain_spec.fulu_fork_epoch = Some(Epoch::new(1401280)); assert_eq!( encode_then_decode_response( @@ -1564,12 +1626,14 @@ mod tests { assert_eq!( encode_then_decode_response( SupportedProtocol::BlocksByRangeV2, - RpcResponse::Success(RpcSuccessResponse::BlocksByRange(Arc::new(altair_block()))), + RpcResponse::Success(RpcSuccessResponse::BlocksByRange(Arc::new(altair_block( + &chain_spec + )))), ForkName::Altair, &chain_spec, ), Ok(Some(RpcSuccessResponse::BlocksByRange(Arc::new( - altair_block() + altair_block(&chain_spec) )))) ); @@ -1642,12 +1706,14 @@ mod tests { assert_eq!( encode_then_decode_response( SupportedProtocol::BlocksByRootV2, - RpcResponse::Success(RpcSuccessResponse::BlocksByRoot(Arc::new(altair_block()))), + RpcResponse::Success(RpcSuccessResponse::BlocksByRoot(Arc::new(altair_block( + &chain_spec + )))), ForkName::Altair, &chain_spec, ), Ok(Some(RpcSuccessResponse::BlocksByRoot(Arc::new( - altair_block() + altair_block(&chain_spec) )))) ); @@ -1810,7 +1876,9 @@ mod tests { // Trying to decode an altair block with base context bytes should give ssz decoding error let mut encoded_bytes = encode_response( SupportedProtocol::BlocksByRootV2, - RpcResponse::Success(RpcSuccessResponse::BlocksByRoot(Arc::new(altair_block()))), + RpcResponse::Success(RpcSuccessResponse::BlocksByRoot(Arc::new(altair_block( + &chain_spec, + )))), ForkName::Altair, &chain_spec, ) @@ -2024,7 +2092,7 @@ mod tests { let malicious_padding: &'static [u8] = b"\xFE\x00\x00\x00"; // Full altair block is 157916 bytes uncompressed. `max_compressed_len` is 32 + 157916 + 157916/6 = 184267. - let block_message_bytes = altair_block().as_ssz_bytes(); + let block_message_bytes = altair_block(&fork_context.spec).as_ssz_bytes(); assert_eq!(block_message_bytes.len(), 157916); assert_eq!( From 3ffbe23e9e0db8a864c2aba31fa48cf739d34eb4 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 9 Jul 2025 22:28:07 +1000 Subject: [PATCH 05/10] Fix moar tests. --- beacon_node/lighthouse_network/src/rpc/codec.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beacon_node/lighthouse_network/src/rpc/codec.rs b/beacon_node/lighthouse_network/src/rpc/codec.rs index aa54db5b4af..014493ec0e8 100644 --- a/beacon_node/lighthouse_network/src/rpc/codec.rs +++ b/beacon_node/lighthouse_network/src/rpc/codec.rs @@ -2120,11 +2120,11 @@ mod tests { dst.extend_from_slice(malicious_padding); } - // Insert payload (8103 bytes compressed) + // Insert payload (8102 bytes compressed) let mut writer = FrameEncoder::new(Vec::new()); writer.write_all(&block_message_bytes).unwrap(); writer.flush().unwrap(); - assert_eq!(writer.get_ref().len(), 8103); + assert_eq!(writer.get_ref().len(), 8102); dst.extend_from_slice(writer.get_ref()); let chain_spec = Spec::default_spec(); From cb2fa443d993463db64a4ae6cf5a5dba855e2f5a Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 9 Jul 2025 23:04:21 +1000 Subject: [PATCH 06/10] Ensure the same `ChainSpec` is used throughout the tests. --- .../lighthouse_network/src/rpc/codec.rs | 186 ++++++++++-------- 1 file changed, 99 insertions(+), 87 deletions(-) diff --git a/beacon_node/lighthouse_network/src/rpc/codec.rs b/beacon_node/lighthouse_network/src/rpc/codec.rs index 014493ec0e8..482dbb1f763 100644 --- a/beacon_node/lighthouse_network/src/rpc/codec.rs +++ b/beacon_node/lighthouse_network/src/rpc/codec.rs @@ -910,46 +910,59 @@ mod tests { type Spec = types::MainnetEthSpec; - fn fork_context(fork_name: ForkName) -> ForkContext { + fn spec_with_all_forks_enabled() -> ChainSpec { let mut chain_spec = Spec::default_spec(); - let altair_fork_epoch = Epoch::new(1); - let bellatrix_fork_epoch = Epoch::new(2); - let capella_fork_epoch = Epoch::new(3); - let deneb_fork_epoch = Epoch::new(4); - let electra_fork_epoch = Epoch::new(5); - let fulu_fork_epoch = Epoch::new(6); - - chain_spec.altair_fork_epoch = Some(altair_fork_epoch); - chain_spec.bellatrix_fork_epoch = Some(bellatrix_fork_epoch); - chain_spec.capella_fork_epoch = Some(capella_fork_epoch); - chain_spec.deneb_fork_epoch = Some(deneb_fork_epoch); - chain_spec.electra_fork_epoch = Some(electra_fork_epoch); - chain_spec.fulu_fork_epoch = Some(fulu_fork_epoch); + chain_spec.altair_fork_epoch = Some(Epoch::new(1)); + chain_spec.bellatrix_fork_epoch = Some(Epoch::new(2)); + chain_spec.capella_fork_epoch = Some(Epoch::new(3)); + chain_spec.deneb_fork_epoch = Some(Epoch::new(4)); + chain_spec.electra_fork_epoch = Some(Epoch::new(5)); + chain_spec.fulu_fork_epoch = Some(Epoch::new(6)); + chain_spec + } + fn fork_context(fork_name: ForkName, spec: &ChainSpec) -> ForkContext { let current_slot = match fork_name { ForkName::Base => Slot::new(0), - ForkName::Altair => altair_fork_epoch.start_slot(Spec::slots_per_epoch()), - ForkName::Bellatrix => bellatrix_fork_epoch.start_slot(Spec::slots_per_epoch()), - ForkName::Capella => capella_fork_epoch.start_slot(Spec::slots_per_epoch()), - ForkName::Deneb => deneb_fork_epoch.start_slot(Spec::slots_per_epoch()), - ForkName::Electra => electra_fork_epoch.start_slot(Spec::slots_per_epoch()), - ForkName::Fulu => fulu_fork_epoch.start_slot(Spec::slots_per_epoch()), + ForkName::Altair => spec + .altair_fork_epoch + .unwrap() + .start_slot(Spec::slots_per_epoch()), + ForkName::Bellatrix => spec + .bellatrix_fork_epoch + .unwrap() + .start_slot(Spec::slots_per_epoch()), + ForkName::Capella => spec + .capella_fork_epoch + .unwrap() + .start_slot(Spec::slots_per_epoch()), + ForkName::Deneb => spec + .deneb_fork_epoch + .unwrap() + .start_slot(Spec::slots_per_epoch()), + ForkName::Electra => spec + .electra_fork_epoch + .unwrap() + .start_slot(Spec::slots_per_epoch()), + ForkName::Fulu => spec + .fulu_fork_epoch + .unwrap() + .start_slot(Spec::slots_per_epoch()), }; - ForkContext::new::(current_slot, Hash256::zero(), &chain_spec) + ForkContext::new::(current_slot, Hash256::zero(), &spec) } /// Smallest sized block across all current forks. Useful for testing /// min length check conditions. - fn empty_base_block() -> SignedBeaconBlock { - let empty_block = BeaconBlock::Base(BeaconBlockBase::::empty(&Spec::default_spec())); + fn empty_base_block(spec: &ChainSpec) -> SignedBeaconBlock { + let empty_block = BeaconBlock::Base(BeaconBlockBase::::empty(spec)); SignedBeaconBlock::from_block(empty_block, Signature::empty()) } fn altair_block(spec: &ChainSpec) -> SignedBeaconBlock { // The context bytes are now derived from the block epoch, so we need to have the slot set // here. - let mut full_block = - BeaconBlock::Altair(BeaconBlockAltair::::full(&Spec::default_spec())); + let mut full_block = BeaconBlock::Altair(BeaconBlockAltair::::full(spec)); *full_block.slot_mut() = spec .altair_fork_epoch .expect("altair fork epoch must be set") @@ -1079,7 +1092,7 @@ mod tests { } } - fn dcbroot_request(spec: &ChainSpec, fork_name: ForkName) -> DataColumnsByRootRequest { + fn dcbroot_request(fork_name: ForkName, spec: &ChainSpec) -> DataColumnsByRootRequest { let number_of_columns = spec.number_of_columns as usize; DataColumnsByRootRequest { data_column_ids: RuntimeVariableList::new( @@ -1093,21 +1106,21 @@ mod tests { } } - fn bbroot_request_v1(fork_name: ForkName) -> BlocksByRootRequest { - BlocksByRootRequest::new_v1(vec![Hash256::zero()], &fork_context(fork_name)) + fn bbroot_request_v1(fork_name: ForkName, spec: &ChainSpec) -> BlocksByRootRequest { + BlocksByRootRequest::new_v1(vec![Hash256::zero()], &fork_context(fork_name, &spec)) } - fn bbroot_request_v2(fork_name: ForkName) -> BlocksByRootRequest { - BlocksByRootRequest::new(vec![Hash256::zero()], &fork_context(fork_name)) + fn bbroot_request_v2(fork_name: ForkName, spec: &ChainSpec) -> BlocksByRootRequest { + BlocksByRootRequest::new(vec![Hash256::zero()], &fork_context(fork_name, &spec)) } - fn blbroot_request(fork_name: ForkName) -> BlobsByRootRequest { + fn blbroot_request(fork_name: ForkName, spec: &ChainSpec) -> BlobsByRootRequest { BlobsByRootRequest::new( vec![BlobIdentifier { block_root: Hash256::zero(), index: 0, }], - &fork_context(fork_name), + &fork_context(fork_name, &spec), ) } @@ -1150,7 +1163,7 @@ mod tests { spec: &ChainSpec, ) -> Result { let snappy_protocol_id = ProtocolId::new(protocol, Encoding::SSZSnappy); - let fork_context = Arc::new(fork_context(fork_name)); + let fork_context = Arc::new(fork_context(fork_name, spec)); let max_packet_size = spec.max_payload_size as usize; let mut buf = BytesMut::new(); @@ -1164,8 +1177,9 @@ mod tests { fn encode_without_length_checks( bytes: Vec, fork_name: ForkName, + spec: &ChainSpec, ) -> Result { - let fork_context = fork_context(fork_name); + let fork_context = fork_context(fork_name, &spec); let mut dst = BytesMut::new(); // Add context bytes if required @@ -1197,7 +1211,7 @@ mod tests { spec: &ChainSpec, ) -> Result>, RPCError> { let snappy_protocol_id = ProtocolId::new(protocol, Encoding::SSZSnappy); - let fork_context = Arc::new(fork_context(fork_name)); + let fork_context = Arc::new(fork_context(fork_name, &spec)); let max_packet_size = spec.max_payload_size as usize; let mut snappy_outbound_codec = SSZSnappyOutboundCodec::::new(snappy_protocol_id, max_packet_size, fork_context); @@ -1218,7 +1232,7 @@ mod tests { /// Verifies that requests we send are encoded in a way that we would correctly decode too. fn encode_then_decode_request(req: RequestType, fork_name: ForkName, spec: &ChainSpec) { - let fork_context = Arc::new(fork_context(fork_name)); + let fork_context = Arc::new(fork_context(fork_name, &spec)); let max_packet_size = spec.max_payload_size as usize; let protocol = ProtocolId::new(req.versioned_protocol(), Encoding::SSZSnappy); // Encode a request we send @@ -1289,9 +1303,7 @@ mod tests { // Test RPCResponse encoding/decoding for V1 messages #[test] fn test_encode_then_decode_v1() { - let mut chain_spec = Spec::default_spec(); - // Set a fulu fork epoch so we can encode / decode data columns - chain_spec.fulu_fork_epoch = Some(Epoch::new(1401280)); + let chain_spec = spec_with_all_forks_enabled(); assert_eq!( encode_then_decode_response( @@ -1328,13 +1340,13 @@ mod tests { encode_then_decode_response( SupportedProtocol::BlocksByRangeV1, RpcResponse::Success(RpcSuccessResponse::BlocksByRange(Arc::new( - empty_base_block() + empty_base_block(&chain_spec) ))), ForkName::Base, &chain_spec, ), Ok(Some(RpcSuccessResponse::BlocksByRange(Arc::new( - empty_base_block() + empty_base_block(&chain_spec) )))) ); @@ -1358,13 +1370,13 @@ mod tests { encode_then_decode_response( SupportedProtocol::BlocksByRootV1, RpcResponse::Success(RpcSuccessResponse::BlocksByRoot(Arc::new( - empty_base_block() + empty_base_block(&chain_spec) ))), ForkName::Base, &chain_spec, ), Ok(Some(RpcSuccessResponse::BlocksByRoot(Arc::new( - empty_base_block() + empty_base_block(&chain_spec) )))) ); @@ -1588,21 +1600,19 @@ mod tests { // Test RPCResponse encoding/decoding for V1 messages #[test] fn test_encode_then_decode_v2() { - let mut chain_spec = Spec::default_spec(); - // Set a fulu fork epoch so we can encode / decode data columns - chain_spec.fulu_fork_epoch = Some(Epoch::new(1401280)); + let chain_spec = spec_with_all_forks_enabled(); assert_eq!( encode_then_decode_response( SupportedProtocol::BlocksByRangeV2, RpcResponse::Success(RpcSuccessResponse::BlocksByRange(Arc::new( - empty_base_block() + empty_base_block(&chain_spec) ))), ForkName::Base, &chain_spec, ), Ok(Some(RpcSuccessResponse::BlocksByRange(Arc::new( - empty_base_block() + empty_base_block(&chain_spec) )))) ); @@ -1613,13 +1623,13 @@ mod tests { encode_then_decode_response( SupportedProtocol::BlocksByRangeV2, RpcResponse::Success(RpcSuccessResponse::BlocksByRange(Arc::new( - empty_base_block() + empty_base_block(&chain_spec) ))), ForkName::Altair, &chain_spec, ), Ok(Some(RpcSuccessResponse::BlocksByRange(Arc::new( - empty_base_block() + empty_base_block(&chain_spec) )))) ); @@ -1654,9 +1664,12 @@ mod tests { )))) ); - let mut encoded = - encode_without_length_checks(bellatrix_block_large.as_ssz_bytes(), ForkName::Bellatrix) - .unwrap(); + let mut encoded = encode_without_length_checks( + bellatrix_block_large.as_ssz_bytes(), + ForkName::Bellatrix, + &chain_spec, + ) + .unwrap(); assert!( matches!( @@ -1676,13 +1689,13 @@ mod tests { encode_then_decode_response( SupportedProtocol::BlocksByRootV2, RpcResponse::Success(RpcSuccessResponse::BlocksByRoot(Arc::new( - empty_base_block() + empty_base_block(&chain_spec) ))), ForkName::Base, &chain_spec, ), Ok(Some(RpcSuccessResponse::BlocksByRoot(Arc::new( - empty_base_block() + empty_base_block(&chain_spec) )))), ); @@ -1693,13 +1706,13 @@ mod tests { encode_then_decode_response( SupportedProtocol::BlocksByRootV2, RpcResponse::Success(RpcSuccessResponse::BlocksByRoot(Arc::new( - empty_base_block() + empty_base_block(&chain_spec) ))), ForkName::Altair, &chain_spec, ), Ok(Some(RpcSuccessResponse::BlocksByRoot(Arc::new( - empty_base_block() + empty_base_block(&chain_spec) )))) ); @@ -1731,9 +1744,12 @@ mod tests { )))) ); - let mut encoded = - encode_without_length_checks(bellatrix_block_large.as_ssz_bytes(), ForkName::Bellatrix) - .unwrap(); + let mut encoded = encode_without_length_checks( + bellatrix_block_large.as_ssz_bytes(), + ForkName::Bellatrix, + &chain_spec, + ) + .unwrap(); assert!( matches!( @@ -1795,15 +1811,14 @@ mod tests { // Test RPCResponse encoding/decoding for V2 messages #[test] fn test_context_bytes_v2() { - let fork_context = fork_context(ForkName::Altair); - - let chain_spec = Spec::default_spec(); + let chain_spec = spec_with_all_forks_enabled(); + let fork_context = fork_context(ForkName::Altair, &chain_spec); // Removing context bytes for v2 messages should error let mut encoded_bytes = encode_response( SupportedProtocol::BlocksByRangeV2, RpcResponse::Success(RpcSuccessResponse::BlocksByRange(Arc::new( - empty_base_block(), + empty_base_block(&chain_spec), ))), ForkName::Base, &chain_spec, @@ -1826,7 +1841,7 @@ mod tests { let mut encoded_bytes = encode_response( SupportedProtocol::BlocksByRootV2, RpcResponse::Success(RpcSuccessResponse::BlocksByRoot(Arc::new( - empty_base_block(), + empty_base_block(&chain_spec), ))), ForkName::Base, &chain_spec, @@ -1850,7 +1865,7 @@ mod tests { let mut encoded_bytes = encode_response( SupportedProtocol::BlocksByRangeV2, RpcResponse::Success(RpcSuccessResponse::BlocksByRange(Arc::new( - empty_base_block(), + empty_base_block(&chain_spec), ))), ForkName::Altair, &chain_spec, @@ -1926,7 +1941,7 @@ mod tests { let mut encoded_bytes = encode_response( SupportedProtocol::BlocksByRootV2, RpcResponse::Success(RpcSuccessResponse::BlocksByRoot(Arc::new( - empty_base_block(), + empty_base_block(&chain_spec), ))), ForkName::Altair, &chain_spec, @@ -1952,7 +1967,7 @@ mod tests { let mut encoded_bytes = encode_response( SupportedProtocol::BlocksByRootV2, RpcResponse::Success(RpcSuccessResponse::BlocksByRoot(Arc::new( - empty_base_block(), + empty_base_block(&chain_spec), ))), ForkName::Altair, &chain_spec, @@ -1974,8 +1989,7 @@ mod tests { #[test] fn test_encode_then_decode_request() { - let fork_context = fork_context(ForkName::Electra); - let chain_spec = fork_context.spec.clone(); + let chain_spec = spec_with_all_forks_enabled(); let requests: &[RequestType] = &[ RequestType::Ping(ping_message()), @@ -1999,10 +2013,10 @@ mod tests { // Handled separately to have consistent `ForkName` across request and responses let fork_dependent_requests = |fork_name| { [ - RequestType::BlobsByRoot(blbroot_request(fork_name)), - RequestType::BlocksByRoot(bbroot_request_v1(fork_name)), - RequestType::BlocksByRoot(bbroot_request_v2(fork_name)), - RequestType::DataColumnsByRoot(dcbroot_request(&chain_spec, fork_name)), + RequestType::BlobsByRoot(blbroot_request(fork_name, &chain_spec)), + RequestType::BlocksByRoot(bbroot_request_v1(fork_name, &chain_spec)), + RequestType::BlocksByRoot(bbroot_request_v2(fork_name, &chain_spec)), + RequestType::DataColumnsByRoot(dcbroot_request(fork_name, &chain_spec)), ] }; for fork_name in ForkName::list_all() { @@ -2062,7 +2076,7 @@ mod tests { assert_eq!(writer.get_ref().len(), 42); dst.extend_from_slice(writer.get_ref()); - let chain_spec = Spec::default_spec(); + let chain_spec = spec_with_all_forks_enabled(); // 10 (for stream identifier) + 80 + 42 = 132 > `max_compressed_len`. Hence, decoding should fail with `InvalidData`. assert!(matches!( decode_response( @@ -2080,7 +2094,8 @@ mod tests { /// sends a valid message filled with a stream of useless padding before the actual message. #[test] fn test_decode_malicious_v2_message() { - let fork_context = Arc::new(fork_context(ForkName::Altair)); + let chain_spec = spec_with_all_forks_enabled(); + let fork_context = Arc::new(fork_context(ForkName::Altair, &chain_spec)); // 10 byte snappy stream identifier let stream_identifier: &'static [u8] = b"\xFF\x06\x00\x00sNaPpY"; @@ -2127,7 +2142,7 @@ mod tests { assert_eq!(writer.get_ref().len(), 8102); dst.extend_from_slice(writer.get_ref()); - let chain_spec = Spec::default_spec(); + let chain_spec = spec_with_all_forks_enabled(); // 10 (for stream identifier) + 176156 + 8103 = 184269 > `max_compressed_len`. Hence, decoding should fail with `InvalidData`. assert!(matches!( @@ -2163,7 +2178,7 @@ mod tests { let mut uvi_codec: Uvi = Uvi::default(); let mut dst = BytesMut::with_capacity(1024); - let chain_spec = Spec::default_spec(); + let chain_spec = spec_with_all_forks_enabled(); // Insert length-prefix uvi_codec @@ -2199,9 +2214,8 @@ mod tests { let snappy_protocol_id = ProtocolId::new(SupportedProtocol::StatusV1, Encoding::SSZSnappy); - let fork_context = Arc::new(fork_context(ForkName::Base)); - - let chain_spec = Spec::default_spec(); + let chain_spec = spec_with_all_forks_enabled(); + let fork_context = Arc::new(fork_context(ForkName::Base, &chain_spec)); let mut snappy_outbound_codec = SSZSnappyOutboundCodec::::new( snappy_protocol_id, @@ -2235,9 +2249,8 @@ mod tests { let snappy_protocol_id = ProtocolId::new(SupportedProtocol::StatusV1, Encoding::SSZSnappy); - let fork_context = Arc::new(fork_context(ForkName::Base)); - - let chain_spec = Spec::default_spec(); + let chain_spec = spec_with_all_forks_enabled(); + let fork_context = Arc::new(fork_context(ForkName::Base, &chain_spec)); let mut snappy_outbound_codec = SSZSnappyOutboundCodec::::new( snappy_protocol_id, @@ -2266,9 +2279,8 @@ mod tests { let protocol_id = ProtocolId::new(SupportedProtocol::BlocksByRangeV1, Encoding::SSZSnappy); // Response limits - let fork_context = Arc::new(fork_context(ForkName::Base)); - - let chain_spec = Spec::default_spec(); + let chain_spec = spec_with_all_forks_enabled(); + let fork_context = Arc::new(fork_context(ForkName::Base, &chain_spec)); let max_rpc_size = chain_spec.max_payload_size as usize; let limit = protocol_id.rpc_response_limits::(&fork_context); From a4d0680bd23eb3e3eec82d1df68a9cec78e36e98 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 10 Jul 2025 01:22:22 +1000 Subject: [PATCH 07/10] Refactor `ForkContext` to avoid recomputing fork digests and using cached values. --- beacon_node/lighthouse_network/src/config.rs | 2 +- .../lighthouse_network/src/rpc/codec.rs | 20 +- .../lighthouse_network/src/rpc/handler.rs | 14 +- .../lighthouse_network/src/rpc/methods.rs | 6 +- .../lighthouse_network/src/rpc/protocol.rs | 16 +- .../src/rpc/rate_limiter.rs | 5 +- .../lighthouse_network/src/service/mod.rs | 2 +- .../lighthouse_network/src/types/pubsub.rs | 169 ++++++------ beacon_node/network/src/service.rs | 12 +- beacon_node/network/src/sync/manager.rs | 2 +- .../network/src/sync/network_context.rs | 2 +- consensus/types/src/chain_spec.rs | 2 +- consensus/types/src/fork_context.rs | 259 ++++++++++++++---- 13 files changed, 333 insertions(+), 178 deletions(-) diff --git a/beacon_node/lighthouse_network/src/config.rs b/beacon_node/lighthouse_network/src/config.rs index bd72a5d51a2..aee53a469c4 100644 --- a/beacon_node/lighthouse_network/src/config.rs +++ b/beacon_node/lighthouse_network/src/config.rs @@ -457,7 +457,7 @@ pub fn gossipsub_config( ) -> Vec { let topic_bytes = message.topic.as_str().as_bytes(); - if fork_context.current_fork().altair_enabled() { + if fork_context.current_fork_name().altair_enabled() { let topic_len_bytes = topic_bytes.len().to_le_bytes(); let mut vec = Vec::with_capacity( prefix.len() + topic_len_bytes.len() + topic_bytes.len() + message.data.len(), diff --git a/beacon_node/lighthouse_network/src/rpc/codec.rs b/beacon_node/lighthouse_network/src/rpc/codec.rs index 482dbb1f763..67ca76fb8a3 100644 --- a/beacon_node/lighthouse_network/src/rpc/codec.rs +++ b/beacon_node/lighthouse_network/src/rpc/codec.rs @@ -193,7 +193,7 @@ impl Decoder for SSZSnappyInboundCodec { handle_rpc_request( self.protocol.versioned_protocol, &decoded_buffer, - self.fork_context.current_fork(), + self.fork_context.current_fork_name(), &self.fork_context.spec, ) } @@ -882,7 +882,7 @@ fn context_bytes_to_fork_name( fork_context: Arc, ) -> Result { fork_context - .from_context_bytes(context_bytes) + .get_fork_from_context_bytes(context_bytes) .cloned() .ok_or_else(|| { let encoded = hex::encode(context_bytes); @@ -949,7 +949,7 @@ mod tests { .unwrap() .start_slot(Spec::slots_per_epoch()), }; - ForkContext::new::(current_slot, Hash256::zero(), &spec) + ForkContext::new::(current_slot, Hash256::zero(), spec) } /// Smallest sized block across all current forks. Useful for testing @@ -1107,11 +1107,11 @@ mod tests { } fn bbroot_request_v1(fork_name: ForkName, spec: &ChainSpec) -> BlocksByRootRequest { - BlocksByRootRequest::new_v1(vec![Hash256::zero()], &fork_context(fork_name, &spec)) + BlocksByRootRequest::new_v1(vec![Hash256::zero()], &fork_context(fork_name, spec)) } fn bbroot_request_v2(fork_name: ForkName, spec: &ChainSpec) -> BlocksByRootRequest { - BlocksByRootRequest::new(vec![Hash256::zero()], &fork_context(fork_name, &spec)) + BlocksByRootRequest::new(vec![Hash256::zero()], &fork_context(fork_name, spec)) } fn blbroot_request(fork_name: ForkName, spec: &ChainSpec) -> BlobsByRootRequest { @@ -1120,7 +1120,7 @@ mod tests { block_root: Hash256::zero(), index: 0, }], - &fork_context(fork_name, &spec), + &fork_context(fork_name, spec), ) } @@ -1179,11 +1179,11 @@ mod tests { fork_name: ForkName, spec: &ChainSpec, ) -> Result { - let fork_context = fork_context(fork_name, &spec); + let fork_context = fork_context(fork_name, spec); let mut dst = BytesMut::new(); // Add context bytes if required - dst.extend_from_slice(&fork_context.context_bytes(fork_context.digest_epoch())); + dst.extend_from_slice(&fork_context.context_bytes(fork_context.current_fork_epoch())); let mut uvi_codec: Uvi = Uvi::default(); @@ -1211,7 +1211,7 @@ mod tests { spec: &ChainSpec, ) -> Result>, RPCError> { let snappy_protocol_id = ProtocolId::new(protocol, Encoding::SSZSnappy); - let fork_context = Arc::new(fork_context(fork_name, &spec)); + let fork_context = Arc::new(fork_context(fork_name, spec)); let max_packet_size = spec.max_payload_size as usize; let mut snappy_outbound_codec = SSZSnappyOutboundCodec::::new(snappy_protocol_id, max_packet_size, fork_context); @@ -1232,7 +1232,7 @@ mod tests { /// Verifies that requests we send are encoded in a way that we would correctly decode too. fn encode_then_decode_request(req: RequestType, fork_name: ForkName, spec: &ChainSpec) { - let fork_context = Arc::new(fork_context(fork_name, &spec)); + let fork_context = Arc::new(fork_context(fork_name, spec)); let max_packet_size = spec.max_payload_size as usize; let protocol = ProtocolId::new(req.versioned_protocol(), Encoding::SSZSnappy); // Encode a request we send diff --git a/beacon_node/lighthouse_network/src/rpc/handler.rs b/beacon_node/lighthouse_network/src/rpc/handler.rs index 823416b8e8d..fe7be936622 100644 --- a/beacon_node/lighthouse_network/src/rpc/handler.rs +++ b/beacon_node/lighthouse_network/src/rpc/handler.rs @@ -912,7 +912,7 @@ where } let (req, substream) = substream; - let current_fork = self.fork_context.current_fork(); + let current_fork = self.fork_context.current_fork_name(); let spec = &self.fork_context.spec; match &req { @@ -950,8 +950,10 @@ where _ => {} }; - let max_responses = - req.max_responses(self.fork_context.digest_epoch(), &self.fork_context.spec); + let max_responses = req.max_responses( + self.fork_context.current_fork_epoch(), + &self.fork_context.spec, + ); // store requests that expect responses if max_responses > 0 { @@ -1021,8 +1023,10 @@ where } // add the stream to substreams if we expect a response, otherwise drop the stream. - let max_responses = - request.max_responses(self.fork_context.digest_epoch(), &self.fork_context.spec); + let max_responses = request.max_responses( + self.fork_context.current_fork_epoch(), + &self.fork_context.spec, + ); if max_responses > 0 { let max_remaining_chunks = if request.expect_exactly_one_response() { // Currently enforced only for multiple responses diff --git a/beacon_node/lighthouse_network/src/rpc/methods.rs b/beacon_node/lighthouse_network/src/rpc/methods.rs index 8e065ba6e53..53005448211 100644 --- a/beacon_node/lighthouse_network/src/rpc/methods.rs +++ b/beacon_node/lighthouse_network/src/rpc/methods.rs @@ -484,7 +484,7 @@ impl BlocksByRootRequest { pub fn new(block_roots: Vec, fork_context: &ForkContext) -> Self { let max_request_blocks = fork_context .spec - .max_request_blocks(fork_context.current_fork()); + .max_request_blocks(fork_context.current_fork_name()); let block_roots = RuntimeVariableList::from_vec(block_roots, max_request_blocks); Self::V2(BlocksByRootRequestV2 { block_roots }) } @@ -492,7 +492,7 @@ impl BlocksByRootRequest { pub fn new_v1(block_roots: Vec, fork_context: &ForkContext) -> Self { let max_request_blocks = fork_context .spec - .max_request_blocks(fork_context.current_fork()); + .max_request_blocks(fork_context.current_fork_name()); let block_roots = RuntimeVariableList::from_vec(block_roots, max_request_blocks); Self::V1(BlocksByRootRequestV1 { block_roots }) } @@ -509,7 +509,7 @@ impl BlobsByRootRequest { pub fn new(blob_ids: Vec, fork_context: &ForkContext) -> Self { let max_request_blob_sidecars = fork_context .spec - .max_request_blob_sidecars(fork_context.current_fork()); + .max_request_blob_sidecars(fork_context.current_fork_name()); let blob_ids = RuntimeVariableList::from_vec(blob_ids, max_request_blob_sidecars); Self { blob_ids } } diff --git a/beacon_node/lighthouse_network/src/rpc/protocol.rs b/beacon_node/lighthouse_network/src/rpc/protocol.rs index 9b502384108..500e98d5c33 100644 --- a/beacon_node/lighthouse_network/src/rpc/protocol.rs +++ b/beacon_node/lighthouse_network/src/rpc/protocol.rs @@ -545,15 +545,15 @@ impl ProtocolId { ::ssz_fixed_len(), ), Protocol::Goodbye => RpcLimits::new(0, 0), // Goodbye request has no response - Protocol::BlocksByRange => rpc_block_limits_by_fork(fork_context.current_fork()), - Protocol::BlocksByRoot => rpc_block_limits_by_fork(fork_context.current_fork()), + Protocol::BlocksByRange => rpc_block_limits_by_fork(fork_context.current_fork_name()), + Protocol::BlocksByRoot => rpc_block_limits_by_fork(fork_context.current_fork_name()), Protocol::BlobsByRange => rpc_blob_limits::(), Protocol::BlobsByRoot => rpc_blob_limits::(), Protocol::DataColumnsByRoot => { - rpc_data_column_limits::(fork_context.digest_epoch(), &fork_context.spec) + rpc_data_column_limits::(fork_context.current_fork_epoch(), &fork_context.spec) } Protocol::DataColumnsByRange => { - rpc_data_column_limits::(fork_context.digest_epoch(), &fork_context.spec) + rpc_data_column_limits::(fork_context.current_fork_epoch(), &fork_context.spec) } Protocol::Ping => RpcLimits::new( ::ssz_fixed_len(), @@ -564,16 +564,16 @@ impl ProtocolId { as Encode>::ssz_fixed_len(), ), Protocol::LightClientBootstrap => { - rpc_light_client_bootstrap_limits_by_fork(fork_context.current_fork()) + rpc_light_client_bootstrap_limits_by_fork(fork_context.current_fork_name()) } Protocol::LightClientOptimisticUpdate => { - rpc_light_client_optimistic_update_limits_by_fork(fork_context.current_fork()) + rpc_light_client_optimistic_update_limits_by_fork(fork_context.current_fork_name()) } Protocol::LightClientFinalityUpdate => { - rpc_light_client_finality_update_limits_by_fork(fork_context.current_fork()) + rpc_light_client_finality_update_limits_by_fork(fork_context.current_fork_name()) } Protocol::LightClientUpdatesByRange => { - rpc_light_client_updates_by_range_limits_by_fork(fork_context.current_fork()) + rpc_light_client_updates_by_range_limits_by_fork(fork_context.current_fork_name()) } } } diff --git a/beacon_node/lighthouse_network/src/rpc/rate_limiter.rs b/beacon_node/lighthouse_network/src/rpc/rate_limiter.rs index c190b4cbf55..f8fd54eb2a9 100644 --- a/beacon_node/lighthouse_network/src/rpc/rate_limiter.rs +++ b/beacon_node/lighthouse_network/src/rpc/rate_limiter.rs @@ -353,7 +353,10 @@ impl RPCRateLimiter { ) -> Result<(), RateLimitedErr> { let time_since_start = self.init_time.elapsed(); let tokens = request - .max_responses(self.fork_context.digest_epoch(), &self.fork_context.spec) + .max_responses( + self.fork_context.current_fork_epoch(), + &self.fork_context.spec, + ) .max(1); let check = diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index 17965fd9ee7..5950941f70b 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -282,7 +282,7 @@ impl Network { // Set up a scoring update interval let update_gossipsub_scores = tokio::time::interval(params.decay_interval); - let current_digest_epoch = ctx.fork_context.digest_epoch(); + let current_digest_epoch = ctx.fork_context.current_fork_epoch(); let current_and_future_digests = ctx.chain_spec .all_digest_epochs() diff --git a/beacon_node/lighthouse_network/src/types/pubsub.rs b/beacon_node/lighthouse_network/src/types/pubsub.rs index 21df75a648c..601c59a9c84 100644 --- a/beacon_node/lighthouse_network/src/types/pubsub.rs +++ b/beacon_node/lighthouse_network/src/types/pubsub.rs @@ -171,28 +171,29 @@ impl PubsubMessage { // the ssz decoders match gossip_topic.kind() { GossipKind::BeaconAggregateAndProof => { - let signed_aggregate_and_proof = - match fork_context.from_context_bytes(gossip_topic.fork_digest) { - Some(&fork_name) => { - if fork_name.electra_enabled() { - SignedAggregateAndProof::Electra( - SignedAggregateAndProofElectra::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ) - } else { - SignedAggregateAndProof::Base( - SignedAggregateAndProofBase::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ) - } - } - None => { - return Err(format!( - "Unknown gossipsub fork digest: {:?}", - gossip_topic.fork_digest - )) + let signed_aggregate_and_proof = match fork_context + .get_fork_from_context_bytes(gossip_topic.fork_digest) + { + Some(&fork_name) => { + if fork_name.electra_enabled() { + SignedAggregateAndProof::Electra( + SignedAggregateAndProofElectra::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ) + } else { + SignedAggregateAndProof::Base( + SignedAggregateAndProofBase::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ) } - }; + } + None => { + return Err(format!( + "Unknown gossipsub fork digest: {:?}", + gossip_topic.fork_digest + )) + } + }; Ok(PubsubMessage::AggregateAndProofAttestation(Box::new( signed_aggregate_and_proof, ))) @@ -206,48 +207,49 @@ impl PubsubMessage { )))) } GossipKind::BeaconBlock => { - let beacon_block = - match fork_context.from_context_bytes(gossip_topic.fork_digest) { - Some(ForkName::Base) => SignedBeaconBlock::::Base( - SignedBeaconBlockBase::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ), - Some(ForkName::Altair) => SignedBeaconBlock::::Altair( - SignedBeaconBlockAltair::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ), - Some(ForkName::Bellatrix) => SignedBeaconBlock::::Bellatrix( - SignedBeaconBlockBellatrix::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ), - Some(ForkName::Capella) => SignedBeaconBlock::::Capella( - SignedBeaconBlockCapella::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ), - Some(ForkName::Deneb) => SignedBeaconBlock::::Deneb( - SignedBeaconBlockDeneb::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ), - Some(ForkName::Electra) => SignedBeaconBlock::::Electra( - SignedBeaconBlockElectra::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ), - Some(ForkName::Fulu) => SignedBeaconBlock::::Fulu( - SignedBeaconBlockFulu::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ), - None => { - return Err(format!( - "Unknown gossipsub fork digest: {:?}", - gossip_topic.fork_digest - )) - } - }; + let beacon_block = match fork_context + .get_fork_from_context_bytes(gossip_topic.fork_digest) + { + Some(ForkName::Base) => SignedBeaconBlock::::Base( + SignedBeaconBlockBase::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ), + Some(ForkName::Altair) => SignedBeaconBlock::::Altair( + SignedBeaconBlockAltair::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ), + Some(ForkName::Bellatrix) => SignedBeaconBlock::::Bellatrix( + SignedBeaconBlockBellatrix::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ), + Some(ForkName::Capella) => SignedBeaconBlock::::Capella( + SignedBeaconBlockCapella::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ), + Some(ForkName::Deneb) => SignedBeaconBlock::::Deneb( + SignedBeaconBlockDeneb::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ), + Some(ForkName::Electra) => SignedBeaconBlock::::Electra( + SignedBeaconBlockElectra::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ), + Some(ForkName::Fulu) => SignedBeaconBlock::::Fulu( + SignedBeaconBlockFulu::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ), + None => { + return Err(format!( + "Unknown gossipsub fork digest: {:?}", + gossip_topic.fork_digest + )) + } + }; Ok(PubsubMessage::BeaconBlock(Arc::new(beacon_block))) } GossipKind::BlobSidecar(blob_index) => { if let Some(fork_name) = - fork_context.from_context_bytes(gossip_topic.fork_digest) + fork_context.get_fork_from_context_bytes(gossip_topic.fork_digest) { if fork_name.deneb_enabled() { let blob_sidecar = Arc::new( @@ -267,7 +269,7 @@ impl PubsubMessage { )) } GossipKind::DataColumnSidecar(subnet_id) => { - match fork_context.from_context_bytes(gossip_topic.fork_digest) { + match fork_context.get_fork_from_context_bytes(gossip_topic.fork_digest) { Some(fork) if fork.fulu_enabled() => { let col_sidecar = Arc::new( DataColumnSidecar::from_ssz_bytes(data) @@ -295,28 +297,29 @@ impl PubsubMessage { Ok(PubsubMessage::ProposerSlashing(Box::new(proposer_slashing))) } GossipKind::AttesterSlashing => { - let attester_slashing = - match fork_context.from_context_bytes(gossip_topic.fork_digest) { - Some(&fork_name) => { - if fork_name.electra_enabled() { - AttesterSlashing::Electra( - AttesterSlashingElectra::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ) - } else { - AttesterSlashing::Base( - AttesterSlashingBase::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ) - } - } - None => { - return Err(format!( - "Unknown gossipsub fork digest: {:?}", - gossip_topic.fork_digest - )) + let attester_slashing = match fork_context + .get_fork_from_context_bytes(gossip_topic.fork_digest) + { + Some(&fork_name) => { + if fork_name.electra_enabled() { + AttesterSlashing::Electra( + AttesterSlashingElectra::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ) + } else { + AttesterSlashing::Base( + AttesterSlashingBase::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ) } - }; + } + None => { + return Err(format!( + "Unknown gossipsub fork digest: {:?}", + gossip_topic.fork_digest + )) + } + }; Ok(PubsubMessage::AttesterSlashing(Box::new(attester_slashing))) } GossipKind::SignedContributionAndProof => { @@ -343,7 +346,7 @@ impl PubsubMessage { ))) } GossipKind::LightClientFinalityUpdate => { - let light_client_finality_update = match fork_context.from_context_bytes(gossip_topic.fork_digest) { + let light_client_finality_update = match fork_context.get_fork_from_context_bytes(gossip_topic.fork_digest) { Some(&fork_name) => { LightClientFinalityUpdate::from_ssz_bytes(data, fork_name) .map_err(|e| format!("{:?}", e))? @@ -358,7 +361,7 @@ impl PubsubMessage { ))) } GossipKind::LightClientOptimisticUpdate => { - let light_client_optimistic_update = match fork_context.from_context_bytes(gossip_topic.fork_digest) { + let light_client_optimistic_update = match fork_context.get_fork_from_context_bytes(gossip_topic.fork_digest) { Some(&fork_name) => { LightClientOptimisticUpdate::from_ssz_bytes(data, fork_name) .map_err(|e| format!("{:?}", e))? diff --git a/beacon_node/network/src/service.rs b/beacon_node/network/src/service.rs index c3bf98917f3..1bd4f9ee398 100644 --- a/beacon_node/network/src/service.rs +++ b/beacon_node/network/src/service.rs @@ -687,7 +687,7 @@ impl NetworkService { let mut subscribed_topics: Vec = vec![]; for topic_kind in core_topics_to_subscribe::( - self.fork_context.current_fork(), + self.fork_context.current_fork_name(), &self.network_globals.as_topic_config(), &self.fork_context.spec, ) { @@ -827,21 +827,21 @@ impl NetworkService { let new_fork_digest = new_enr_fork_id.fork_digest; let fork_context = &self.fork_context; - if let Some(new_fork_name) = fork_context.from_context_bytes(new_fork_digest) { - if fork_context.current_fork() == *new_fork_name { + if let Some(new_fork_name) = fork_context.get_fork_from_context_bytes(new_fork_digest) { + if fork_context.current_fork_name() == *new_fork_name { info!( epoch = ?current_epoch, "BPO Fork Triggered" ) } else { info!( - old_fork = ?fork_context.current_fork(), + old_fork = ?fork_context.current_fork_name(), new_fork = ?new_fork_name, "Transitioned to new fork" ); } - fork_context.update_digest_epoch(current_epoch); + fork_context.update_current_fork(*new_fork_name, new_fork_digest, current_epoch); if self.beacon_chain.spec.is_peer_das_scheduled() { self.libp2p.update_nfd(fork_context.next_fork_digest()); } @@ -874,7 +874,7 @@ impl NetworkService { fn subscribed_core_topics(&self) -> bool { let core_topics = core_topics_to_subscribe::( - self.fork_context.current_fork(), + self.fork_context.current_fork_name(), &self.network_globals.as_topic_config(), &self.fork_context.spec, ); diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index d11a18ed0ae..81b22b99e89 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -264,7 +264,7 @@ pub fn spawn( fork_context: Arc, ) { assert!( - beacon_chain.spec.max_request_blocks(fork_context.current_fork()) as u64 >= T::EthSpec::slots_per_epoch() * EPOCHS_PER_BATCH, + beacon_chain.spec.max_request_blocks(fork_context.current_fork_name()) as u64 >= T::EthSpec::slots_per_epoch() * EPOCHS_PER_BATCH, "Max blocks that can be requested in a single batch greater than max allowed blocks in a single request" ); diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index d0e62e4ada7..2f74bdc7337 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -879,7 +879,7 @@ impl SyncNetworkContext { request: RequestType::DataColumnsByRoot( request .clone() - .try_into_request(self.fork_context.current_fork(), &self.chain.spec)?, + .try_into_request(self.fork_context.current_fork_name(), &self.chain.spec)?, ), app_request_id: AppRequestId::Sync(SyncRequestId::DataColumnsByRoot(id)), })?; diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 915935b2e82..51b791a0ace 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -246,7 +246,7 @@ pub struct ChainSpec { /* * Networking Fulu */ - blob_schedule: BlobSchedule, + pub(crate) blob_schedule: BlobSchedule, min_epochs_for_data_column_sidecars_requests: u64, /* diff --git a/consensus/types/src/fork_context.rs b/consensus/types/src/fork_context.rs index 8e79b9a8aef..b537e56f80e 100644 --- a/consensus/types/src/fork_context.rs +++ b/consensus/types/src/fork_context.rs @@ -1,17 +1,30 @@ use parking_lot::RwLock; use crate::{ChainSpec, Epoch, EthSpec, ForkName, Hash256, Slot}; -use std::collections::{HashMap, HashSet}; +use std::collections::BTreeMap; + +#[derive(Debug, Clone)] +pub struct HardFork { + fork_name: ForkName, + fork_epoch: Epoch, + fork_digest: [u8; 4], +} + +impl HardFork { + pub fn new(fork_name: ForkName, fork_digest: [u8; 4], fork_epoch: Epoch) -> HardFork { + HardFork { + fork_name, + fork_epoch, + fork_digest, + } + } +} /// Provides fork specific info like the current fork name and the fork digests corresponding to every valid fork. #[derive(Debug)] pub struct ForkContext { - /// Activation epoch of the current hard fork. This can be either a named fork (`ForkName`) or - /// an unnamed blob parameter only fork (BPO). - digest_epoch: RwLock, - enabled_forks: HashSet, - genesis_validators_root: Hash256, - digest_to_fork: HashMap<[u8; 4], ForkName>, + current_fork: RwLock, + epoch_to_forks: BTreeMap, pub spec: ChainSpec, } @@ -25,97 +38,229 @@ impl ForkContext { genesis_validators_root: Hash256, spec: &ChainSpec, ) -> Self { - let enabled_forks = ForkName::list_all() - .into_iter() - .filter(|fork| spec.fork_epoch(*fork).is_some()) - .collect(); - - let epoch_to_digest: HashMap<_, _> = spec + let epoch_to_forks: BTreeMap<_, _> = spec .all_digest_epochs() .map(|epoch| { + let fork_name = spec.fork_name_at_epoch(epoch); let fork_digest = spec.compute_fork_digest(genesis_validators_root, epoch); - (epoch, fork_digest) - }) - .collect(); - - let digest_to_fork = epoch_to_digest - .iter() - .map(|(epoch, digest)| { - let fork_name = spec.fork_name_at_epoch(*epoch); - (*digest, fork_name) + (epoch, HardFork::new(fork_name, fork_digest, epoch)) }) .collect(); let current_epoch = current_slot.epoch(E::slots_per_epoch()); - let digest_epoch = RwLock::new( - epoch_to_digest - .keys() - .filter(|&&epoch| epoch <= current_epoch) - .max() - .cloned() - .expect("should match at least genesis epoch"), - ); + let current_fork = epoch_to_forks + .values() + .filter(|&fork| fork.fork_epoch <= current_epoch) + .next_back() + .cloned() + .expect("should match at least genesis epoch"); Self { - digest_epoch, - enabled_forks, - genesis_validators_root, - digest_to_fork, + current_fork: RwLock::new(current_fork), + epoch_to_forks, spec: spec.clone(), } } /// Returns `true` if the provided `fork_name` exists in the `ForkContext` object. pub fn fork_exists(&self, fork_name: ForkName) -> bool { - self.enabled_forks.contains(&fork_name) + self.spec.fork_epoch(fork_name).is_some() } - /// Returns the `current_fork`. - pub fn current_fork(&self) -> ForkName { - self.spec.fork_name_at_epoch(self.digest_epoch()) + /// Returns the current fork name. + pub fn current_fork_name(&self) -> ForkName { + self.current_fork.read().fork_name } - /// Returns the current digest epoch. - pub fn digest_epoch(&self) -> Epoch { - *self.digest_epoch.read() + /// Returns the current fork epoch. + pub fn current_fork_epoch(&self) -> Epoch { + self.current_fork.read().fork_epoch } + /// Returns the next fork digest. If there's no future fork, returns the current fork digest. pub fn next_fork_digest(&self) -> [u8; 4] { - self.spec - .next_digest_epoch(self.digest_epoch()) - .map(|epoch| { - self.spec - .compute_fork_digest(self.genesis_validators_root, epoch) - }) - .unwrap_or_default() + let current_fork_epoch = self.current_fork_epoch(); + self.epoch_to_forks + .range(current_fork_epoch..) + .nth(1) + .map(|(_, fork)| fork.fork_digest) + .unwrap_or_else(|| self.current_fork.read().fork_digest) } /// Updates the `digest_epoch` field to a new digest epoch. - pub fn update_digest_epoch(&self, epoch: Epoch) { - *self.digest_epoch.write() = epoch; + pub fn update_current_fork( + &self, + new_fork_name: ForkName, + new_fork_digest: [u8; 4], + new_fork_epoch: Epoch, + ) { + debug_assert!(self.epoch_to_forks.contains_key(&new_fork_epoch)); + *self.current_fork.write() = HardFork::new(new_fork_name, new_fork_digest, new_fork_epoch); } /// Returns the context bytes/fork_digest corresponding to the genesis fork version. pub fn genesis_context_bytes(&self) -> [u8; 4] { - self.spec - .compute_fork_digest(self.genesis_validators_root, Epoch::new(0)) + self.epoch_to_forks + .first_key_value() + .expect("must contain genesis epoch") + .1 + .fork_digest } /// Returns the fork type given the context bytes/fork_digest. /// Returns `None` if context bytes doesn't correspond to any valid `ForkName`. - pub fn from_context_bytes(&self, context: [u8; 4]) -> Option<&ForkName> { - self.digest_to_fork.get(&context) + pub fn get_fork_from_context_bytes(&self, context: [u8; 4]) -> Option<&ForkName> { + self.epoch_to_forks + .values() + .find(|fork| fork.fork_digest == context) + .map(|fork| &fork.fork_name) } /// Returns the context bytes/fork_digest corresponding to an epoch. /// See [`ChainSpec::compute_fork_digest`] pub fn context_bytes(&self, epoch: Epoch) -> [u8; 4] { - self.spec - .compute_fork_digest(self.genesis_validators_root, epoch) + self.epoch_to_forks + .range(..=epoch) + .next_back() + .expect("should match at least genesis epoch") + .1 + .fork_digest } /// Returns all `fork_digest`s that are currently in the `ForkContext` object. pub fn all_fork_digests(&self) -> Vec<[u8; 4]> { - self.digest_to_fork.keys().cloned().collect() + self.epoch_to_forks + .values() + .map(|fork| fork.fork_digest) + .collect() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::chain_spec::{BlobParameters, BlobSchedule}; + use crate::MainnetEthSpec; + + type E = MainnetEthSpec; + + fn make_chain_spec() -> ChainSpec { + let blob_parameters = vec![ + BlobParameters { + epoch: Epoch::new(6), + max_blobs_per_block: 12, + }, + BlobParameters { + epoch: Epoch::new(50), + max_blobs_per_block: 24, + }, + BlobParameters { + epoch: Epoch::new(100), + max_blobs_per_block: 48, + }, + ]; + + let mut spec = E::default_spec(); + spec.altair_fork_epoch = Some(Epoch::new(1)); + spec.bellatrix_fork_epoch = Some(Epoch::new(2)); + spec.capella_fork_epoch = Some(Epoch::new(3)); + spec.deneb_fork_epoch = Some(Epoch::new(4)); + spec.electra_fork_epoch = Some(Epoch::new(5)); + spec.fulu_fork_epoch = Some(Epoch::new(6)); + spec.blob_schedule = BlobSchedule::new(blob_parameters); + spec + } + + #[test] + fn test_fork_exists() { + let spec = make_chain_spec(); + let genesis_root = Hash256::ZERO; + let current_slot = Slot::new(7); + + let context = ForkContext::new::(current_slot, genesis_root, &spec); + + assert!(context.fork_exists(ForkName::Electra)); + assert!(context.fork_exists(ForkName::Fulu)); + } + + #[test] + fn test_current_fork_name_and_epoch() { + let spec = make_chain_spec(); + let electra_epoch = spec.electra_fork_epoch.unwrap(); + let electra_slot = electra_epoch.end_slot(E::slots_per_epoch()); + let genesis_root = Hash256::ZERO; + + let context = ForkContext::new::(electra_slot, genesis_root, &spec); + + assert_eq!(context.current_fork_name(), ForkName::Electra); + assert_eq!(context.current_fork_epoch(), electra_epoch); + } + + #[test] + fn test_next_fork_digest() { + let spec = make_chain_spec(); + let electra_epoch = spec.electra_fork_epoch.unwrap(); + let electra_slot = electra_epoch.end_slot(E::slots_per_epoch()); + let genesis_root = Hash256::ZERO; + + let context = ForkContext::new::(electra_slot, genesis_root, &spec); + + let next_digest = context.next_fork_digest(); + let expected_digest = spec.compute_fork_digest(genesis_root, spec.fulu_fork_epoch.unwrap()); + assert_eq!(next_digest, expected_digest); + } + + #[test] + fn test_get_fork_from_context_bytes() { + let spec = make_chain_spec(); + let genesis_root = Hash256::ZERO; + let current_slot = Slot::new(0); + + let context = ForkContext::new::(current_slot, genesis_root, &spec); + + let electra_digest = spec.compute_fork_digest(genesis_root, Epoch::new(5)); + assert_eq!( + context.get_fork_from_context_bytes(electra_digest), + Some(&ForkName::Electra) + ); + + let invalid_digest = [9, 9, 9, 9]; + assert!(context + .get_fork_from_context_bytes(invalid_digest) + .is_none()); + } + + #[test] + fn test_context_bytes() { + let spec = make_chain_spec(); + let genesis_root = Hash256::ZERO; + let current_slot = Slot::new(0); + + let context = ForkContext::new::(current_slot, genesis_root, &spec); + + assert_eq!( + context.context_bytes(Epoch::new(0)), + spec.compute_fork_digest(genesis_root, Epoch::new(0)) + ); + + assert_eq!( + context.context_bytes(Epoch::new(12)), + spec.compute_fork_digest(genesis_root, Epoch::new(10)) + ); + } + + #[test] + fn test_all_fork_digests() { + let spec = make_chain_spec(); + let genesis_root = Hash256::ZERO; + let current_slot = Slot::new(20); + + let context = ForkContext::new::(current_slot, genesis_root, &spec); + + // Get all enabled fork digests + let fork_digests = context.all_fork_digests(); + let expected_digest_count = spec.all_digest_epochs().count(); + + assert_eq!(fork_digests.len(), expected_digest_count); } } From 1be80466effba475635684657c00b68dacd568f2 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 10 Jul 2025 16:46:37 +1000 Subject: [PATCH 08/10] Fix failing tests due to mismatching chainspec and block slots. --- .../lighthouse_network/src/rpc/codec.rs | 49 ++++------------- .../lighthouse_network/tests/common.rs | 52 +++++++++---------- .../lighthouse_network/tests/rpc_tests.rs | 46 +++++++++------- consensus/types/src/beacon_block.rs | 35 ++++++++++--- consensus/types/src/chain_spec.rs | 11 ---- consensus/types/src/signed_beacon_block.rs | 13 ++++- 6 files changed, 103 insertions(+), 103 deletions(-) diff --git a/beacon_node/lighthouse_network/src/rpc/codec.rs b/beacon_node/lighthouse_network/src/rpc/codec.rs index 67ca76fb8a3..186f61b1e96 100644 --- a/beacon_node/lighthouse_network/src/rpc/codec.rs +++ b/beacon_node/lighthouse_network/src/rpc/codec.rs @@ -922,33 +922,16 @@ mod tests { } fn fork_context(fork_name: ForkName, spec: &ChainSpec) -> ForkContext { - let current_slot = match fork_name { - ForkName::Base => Slot::new(0), - ForkName::Altair => spec - .altair_fork_epoch - .unwrap() - .start_slot(Spec::slots_per_epoch()), - ForkName::Bellatrix => spec - .bellatrix_fork_epoch - .unwrap() - .start_slot(Spec::slots_per_epoch()), - ForkName::Capella => spec - .capella_fork_epoch - .unwrap() - .start_slot(Spec::slots_per_epoch()), - ForkName::Deneb => spec - .deneb_fork_epoch - .unwrap() - .start_slot(Spec::slots_per_epoch()), - ForkName::Electra => spec - .electra_fork_epoch - .unwrap() - .start_slot(Spec::slots_per_epoch()), - ForkName::Fulu => spec - .fulu_fork_epoch - .unwrap() - .start_slot(Spec::slots_per_epoch()), + let current_epoch = match fork_name { + ForkName::Base => Some(Epoch::new(0)), + ForkName::Altair => spec.altair_fork_epoch, + ForkName::Bellatrix => spec.bellatrix_fork_epoch, + ForkName::Capella => spec.capella_fork_epoch, + ForkName::Deneb => spec.deneb_fork_epoch, + ForkName::Electra => spec.electra_fork_epoch, + ForkName::Fulu => spec.fulu_fork_epoch, }; + let current_slot = current_epoch.unwrap().start_slot(Spec::slots_per_epoch()); ForkContext::new::(current_slot, Hash256::zero(), spec) } @@ -962,11 +945,7 @@ mod tests { fn altair_block(spec: &ChainSpec) -> SignedBeaconBlock { // The context bytes are now derived from the block epoch, so we need to have the slot set // here. - let mut full_block = BeaconBlock::Altair(BeaconBlockAltair::::full(spec)); - *full_block.slot_mut() = spec - .altair_fork_epoch - .expect("altair fork epoch must be set") - .start_slot(Spec::slots_per_epoch()); + let full_block = BeaconBlock::Altair(BeaconBlockAltair::::full(spec)); SignedBeaconBlock::from_block(full_block, Signature::empty()) } @@ -1010,10 +989,6 @@ mod tests { // here. let mut block: BeaconBlockBellatrix<_, FullPayload> = BeaconBlockBellatrix::empty(spec); - block.slot = spec - .bellatrix_fork_epoch - .expect("Bellatrix epoch must be set") - .start_slot(Spec::slots_per_epoch()); let tx = VariableList::from(vec![0; 1024]); let txs = VariableList::from(std::iter::repeat_n(tx, 5000).collect::>()); @@ -1033,10 +1008,6 @@ mod tests { // here. let mut block: BeaconBlockBellatrix<_, FullPayload> = BeaconBlockBellatrix::empty(spec); - block.slot = spec - .bellatrix_fork_epoch - .expect("Bellatrix epoch must be set") - .start_slot(Spec::slots_per_epoch()); let tx = VariableList::from(vec![0; 1024]); let txs = VariableList::from(std::iter::repeat_n(tx, 100000).collect::>()); diff --git a/beacon_node/lighthouse_network/tests/common.rs b/beacon_node/lighthouse_network/tests/common.rs index 0dac126909c..eb945b61ce2 100644 --- a/beacon_node/lighthouse_network/tests/common.rs +++ b/beacon_node/lighthouse_network/tests/common.rs @@ -11,7 +11,7 @@ use tracing::{debug, error, info_span, Instrument}; use tracing_subscriber::EnvFilter; use types::{ ChainSpec, EnrForkId, Epoch, EthSpec, FixedBytesExtended, ForkContext, ForkName, Hash256, - MinimalEthSpec, Slot, + MinimalEthSpec, }; type E = MinimalEthSpec; @@ -19,33 +19,33 @@ type E = MinimalEthSpec; use lighthouse_network::rpc::config::InboundRateLimiterConfig; use tempfile::Builder as TempBuilder; -/// Returns a dummy fork context -pub fn fork_context(fork_name: ForkName) -> ForkContext { +/// Returns a chain spec with all forks enabled. +pub fn spec_with_all_forks_enabled() -> ChainSpec { let mut chain_spec = E::default_spec(); - let altair_fork_epoch = Epoch::new(1); - let bellatrix_fork_epoch = Epoch::new(2); - let capella_fork_epoch = Epoch::new(3); - let deneb_fork_epoch = Epoch::new(4); - let electra_fork_epoch = Epoch::new(5); - let fulu_fork_epoch = Epoch::new(6); - - chain_spec.altair_fork_epoch = Some(altair_fork_epoch); - chain_spec.bellatrix_fork_epoch = Some(bellatrix_fork_epoch); - chain_spec.capella_fork_epoch = Some(capella_fork_epoch); - chain_spec.deneb_fork_epoch = Some(deneb_fork_epoch); - chain_spec.electra_fork_epoch = Some(electra_fork_epoch); - chain_spec.fulu_fork_epoch = Some(fulu_fork_epoch); + chain_spec.altair_fork_epoch = Some(Epoch::new(1)); + chain_spec.bellatrix_fork_epoch = Some(Epoch::new(2)); + chain_spec.capella_fork_epoch = Some(Epoch::new(3)); + chain_spec.deneb_fork_epoch = Some(Epoch::new(4)); + chain_spec.electra_fork_epoch = Some(Epoch::new(5)); + chain_spec.fulu_fork_epoch = Some(Epoch::new(6)); + chain_spec +} - let current_slot = match fork_name { - ForkName::Base => Slot::new(0), - ForkName::Altair => altair_fork_epoch.start_slot(E::slots_per_epoch()), - ForkName::Bellatrix => bellatrix_fork_epoch.start_slot(E::slots_per_epoch()), - ForkName::Capella => capella_fork_epoch.start_slot(E::slots_per_epoch()), - ForkName::Deneb => deneb_fork_epoch.start_slot(E::slots_per_epoch()), - ForkName::Electra => electra_fork_epoch.start_slot(E::slots_per_epoch()), - ForkName::Fulu => fulu_fork_epoch.start_slot(E::slots_per_epoch()), +/// Returns a dummy fork context +pub fn fork_context(fork_name: ForkName, spec: &ChainSpec) -> ForkContext { + let current_epoch = match fork_name { + ForkName::Base => Some(Epoch::new(0)), + ForkName::Altair => spec.altair_fork_epoch, + ForkName::Bellatrix => spec.bellatrix_fork_epoch, + ForkName::Capella => spec.capella_fork_epoch, + ForkName::Deneb => spec.deneb_fork_epoch, + ForkName::Electra => spec.electra_fork_epoch, + ForkName::Fulu => spec.fulu_fork_epoch, }; - ForkContext::new::(current_slot, Hash256::zero(), &chain_spec) + let current_slot = current_epoch + .unwrap_or_else(|| panic!("expect fork {fork_name} to be scheduled")) + .start_slot(E::slots_per_epoch()); + ForkContext::new::(current_slot, Hash256::zero(), spec) } pub struct Libp2pInstance( @@ -122,7 +122,7 @@ pub async fn build_libp2p_instance( let libp2p_context = lighthouse_network::Context { config, enr_fork_id: EnrForkId::default(), - fork_context: Arc::new(fork_context(fork_name)), + fork_context: Arc::new(fork_context(fork_name, &chain_spec)), chain_spec, libp2p_registry: None, }; diff --git a/beacon_node/lighthouse_network/tests/rpc_tests.rs b/beacon_node/lighthouse_network/tests/rpc_tests.rs index e50f70e43a0..11fe93288f7 100644 --- a/beacon_node/lighthouse_network/tests/rpc_tests.rs +++ b/beacon_node/lighthouse_network/tests/rpc_tests.rs @@ -2,6 +2,7 @@ mod common; +use crate::common::spec_with_all_forks_enabled; use common::{build_tracing_subscriber, Protocol}; use lighthouse_network::rpc::{methods::*, RequestType}; use lighthouse_network::service::api_types::AppRequestId; @@ -60,7 +61,7 @@ fn test_tcp_status_rpc() { let rt = Arc::new(Runtime::new().unwrap()); - let spec = Arc::new(E::default_spec()); + let spec = Arc::new(spec_with_all_forks_enabled()); rt.block_on(async { // get sender/receiver @@ -168,7 +169,7 @@ fn test_tcp_blocks_by_range_chunked_rpc() { let rt = Arc::new(Runtime::new().unwrap()); - let spec = Arc::new(E::default_spec()); + let spec = Arc::new(spec_with_all_forks_enabled()); rt.block_on(async { // get sender/receiver @@ -318,7 +319,7 @@ fn test_blobs_by_range_chunked_rpc() { rt.block_on(async { // get sender/receiver - let spec = Arc::new(E::default_spec()); + let spec = Arc::new(spec_with_all_forks_enabled()); let (mut sender, mut receiver) = common::build_node_pair( Arc::downgrade(&rt), ForkName::Deneb, @@ -330,13 +331,18 @@ fn test_blobs_by_range_chunked_rpc() { .await; // BlobsByRange Request + let deneb_slot = spec + .deneb_fork_epoch + .expect("deneb must be scheduled") + .start_slot(E::slots_per_epoch()); let rpc_request = RequestType::BlobsByRange(BlobsByRangeRequest { - start_slot: 0, + start_slot: deneb_slot.as_u64(), count: slot_count, }); - // BlocksByRange Response - let blob = BlobSidecar::::empty(); + // BlobsByRange Response + let mut blob = BlobSidecar::::empty(); + blob.signed_block_header.message.slot = deneb_slot; let rpc_response = Response::BlobsByRange(Some(Arc::new(blob))); @@ -438,7 +444,7 @@ fn test_tcp_blocks_by_range_over_limit() { let rt = Arc::new(Runtime::new().unwrap()); - let spec = Arc::new(E::default_spec()); + let spec = Arc::new(spec_with_all_forks_enabled()); rt.block_on(async { // get sender/receiver @@ -545,7 +551,7 @@ fn test_tcp_blocks_by_range_chunked_rpc_terminates_correctly() { let rt = Arc::new(Runtime::new().unwrap()); - let spec = Arc::new(E::default_spec()); + let spec = Arc::new(spec_with_all_forks_enabled()); rt.block_on(async { // get sender/receiver @@ -681,7 +687,7 @@ fn test_tcp_blocks_by_range_single_empty_rpc() { let rt = Arc::new(Runtime::new().unwrap()); - let spec = Arc::new(E::default_spec()); + let spec = Arc::new(spec_with_all_forks_enabled()); rt.block_on(async { // get sender/receiver @@ -804,14 +810,15 @@ fn test_tcp_blocks_by_root_chunked_rpc() { let messages_to_send = 6; - let spec = Arc::new(E::default_spec()); + let spec = Arc::new(spec_with_all_forks_enabled()); + let current_fork_name = ForkName::Bellatrix; let rt = Arc::new(Runtime::new().unwrap()); // get sender/receiver rt.block_on(async { let (mut sender, mut receiver) = common::build_node_pair( Arc::downgrade(&rt), - ForkName::Bellatrix, + current_fork_name, spec.clone(), Protocol::Tcp, false, @@ -831,7 +838,7 @@ fn test_tcp_blocks_by_root_chunked_rpc() { Hash256::zero(), Hash256::zero(), ], - spec.max_request_blocks_upper_bound(), + spec.max_request_blocks(current_fork_name), ), })); @@ -934,7 +941,7 @@ fn test_tcp_blocks_by_root_chunked_rpc() { tokio::select! { _ = sender_future => {} _ = receiver_future => {} - _ = sleep(Duration::from_secs(30)) => { + _ = sleep(Duration::from_secs(300)) => { panic!("Future timed out"); } } @@ -952,14 +959,15 @@ fn test_tcp_blocks_by_root_chunked_rpc_terminates_correctly() { let messages_to_send: u64 = 10; let extra_messages_to_send: u64 = 10; - let spec = Arc::new(E::default_spec()); + let spec = Arc::new(spec_with_all_forks_enabled()); + let current_fork = ForkName::Base; let rt = Arc::new(Runtime::new().unwrap()); // get sender/receiver rt.block_on(async { let (mut sender, mut receiver) = common::build_node_pair( Arc::downgrade(&rt), - ForkName::Base, + current_fork, spec.clone(), Protocol::Tcp, false, @@ -983,7 +991,7 @@ fn test_tcp_blocks_by_root_chunked_rpc_terminates_correctly() { Hash256::zero(), Hash256::zero(), ], - spec.max_request_blocks_upper_bound(), + spec.max_request_blocks(current_fork), ), })); @@ -1098,7 +1106,7 @@ fn goodbye_test(log_level: &str, enable_logging: bool, protocol: Protocol) { let rt = Arc::new(Runtime::new().unwrap()); - let spec = Arc::new(E::default_spec()); + let spec = Arc::new(spec_with_all_forks_enabled()); // get sender/receiver rt.block_on(async { @@ -1180,7 +1188,7 @@ fn test_delayed_rpc_response() { // Set up the logging. build_tracing_subscriber("debug", true); let rt = Arc::new(Runtime::new().unwrap()); - let spec = Arc::new(E::default_spec()); + let spec = Arc::new(spec_with_all_forks_enabled()); // Allow 1 token to be use used every 3 seconds. const QUOTA_SEC: u64 = 3; @@ -1314,7 +1322,7 @@ fn test_active_requests() { // Set up the logging. build_tracing_subscriber("debug", true); let rt = Arc::new(Runtime::new().unwrap()); - let spec = Arc::new(E::default_spec()); + let spec = Arc::new(spec_with_all_forks_enabled()); rt.block_on(async { // Get sender/receiver. diff --git a/consensus/types/src/beacon_block.rs b/consensus/types/src/beacon_block.rs index 385cd0fcf51..9168a3feee0 100644 --- a/consensus/types/src/beacon_block.rs +++ b/consensus/types/src/beacon_block.rs @@ -414,7 +414,10 @@ impl> EmptyBlock for BeaconBlockAlta /// Returns an empty Altair block to be used during genesis. fn empty(spec: &ChainSpec) -> Self { BeaconBlockAltair { - slot: spec.genesis_slot, + slot: spec + .altair_fork_epoch + .expect("altair enabled") + .start_slot(E::slots_per_epoch()), proposer_index: 0, parent_root: Hash256::zero(), state_root: Hash256::zero(), @@ -447,7 +450,10 @@ impl> BeaconBlockAltair sync_committee_bits: BitVector::default(), }; BeaconBlockAltair { - slot: spec.genesis_slot, + slot: spec + .altair_fork_epoch + .expect("altair enabled") + .start_slot(E::slots_per_epoch()), proposer_index: 0, parent_root: Hash256::zero(), state_root: Hash256::zero(), @@ -475,7 +481,10 @@ impl> EmptyBlock for BeaconBlockBell /// Returns an empty Bellatrix block to be used during genesis. fn empty(spec: &ChainSpec) -> Self { BeaconBlockBellatrix { - slot: spec.genesis_slot, + slot: spec + .bellatrix_fork_epoch + .expect("bellatrix enabled") + .start_slot(E::slots_per_epoch()), proposer_index: 0, parent_root: Hash256::zero(), state_root: Hash256::zero(), @@ -503,7 +512,10 @@ impl> EmptyBlock for BeaconBlockCape /// Returns an empty Capella block to be used during genesis. fn empty(spec: &ChainSpec) -> Self { BeaconBlockCapella { - slot: spec.genesis_slot, + slot: spec + .capella_fork_epoch + .expect("capella enabled") + .start_slot(E::slots_per_epoch()), proposer_index: 0, parent_root: Hash256::zero(), state_root: Hash256::zero(), @@ -532,7 +544,10 @@ impl> EmptyBlock for BeaconBlockDene /// Returns an empty Deneb block to be used during genesis. fn empty(spec: &ChainSpec) -> Self { BeaconBlockDeneb { - slot: spec.genesis_slot, + slot: spec + .deneb_fork_epoch + .expect("deneb enabled") + .start_slot(E::slots_per_epoch()), proposer_index: 0, parent_root: Hash256::zero(), state_root: Hash256::zero(), @@ -562,7 +577,10 @@ impl> EmptyBlock for BeaconBlockElec /// Returns an empty Electra block to be used during genesis. fn empty(spec: &ChainSpec) -> Self { BeaconBlockElectra { - slot: spec.genesis_slot, + slot: spec + .electra_fork_epoch + .expect("electra enabled") + .start_slot(E::slots_per_epoch()), proposer_index: 0, parent_root: Hash256::zero(), state_root: Hash256::zero(), @@ -593,7 +611,10 @@ impl> EmptyBlock for BeaconBlockFulu /// Returns an empty Fulu block to be used during genesis. fn empty(spec: &ChainSpec) -> Self { BeaconBlockFulu { - slot: spec.genesis_slot, + slot: spec + .fulu_fork_epoch + .expect("fulu enabled") + .start_slot(E::slots_per_epoch()), proposer_index: 0, parent_root: Hash256::zero(), state_root: Hash256::zero(), diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 51b791a0ace..4476cd69b3a 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -676,17 +676,6 @@ impl ChainSpec { } } - /// Returns the highest possible value for max_request_blocks based on enabled forks. - /// - /// This is useful for upper bounds in testing. - pub fn max_request_blocks_upper_bound(&self) -> usize { - if self.deneb_fork_epoch.is_some() { - self.max_request_blocks_deneb as usize - } else { - self.max_request_blocks as usize - } - } - pub fn max_request_blob_sidecars(&self, fork_name: ForkName) -> usize { if fork_name.electra_enabled() { self.max_request_blob_sidecars_electra as usize diff --git a/consensus/types/src/signed_beacon_block.rs b/consensus/types/src/signed_beacon_block.rs index 85bed35a19c..1b5ab365712 100644 --- a/consensus/types/src/signed_beacon_block.rs +++ b/consensus/types/src/signed_beacon_block.rs @@ -883,11 +883,22 @@ mod test { } } + fn spec_with_all_forks_enabled() -> ChainSpec { + let mut chain_spec = E::default_spec(); + chain_spec.altair_fork_epoch = Some(Epoch::new(1)); + chain_spec.bellatrix_fork_epoch = Some(Epoch::new(2)); + chain_spec.capella_fork_epoch = Some(Epoch::new(3)); + chain_spec.deneb_fork_epoch = Some(Epoch::new(4)); + chain_spec.electra_fork_epoch = Some(Epoch::new(5)); + chain_spec.fulu_fork_epoch = Some(Epoch::new(6)); + chain_spec + } + #[test] fn test_ssz_tagged_signed_beacon_block() { type E = MainnetEthSpec; - let spec = &E::default_spec(); + let spec = &spec_with_all_forks_enabled::(); let sig = Signature::empty(); let blocks = vec![ SignedBeaconBlock::::from_block( From 5786207e40165919257870620638f743b7c4a6e6 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 10 Jul 2025 16:59:47 +1000 Subject: [PATCH 09/10] Review comments - adjust code style and add docs. --- .../lighthouse_network/src/service/mod.rs | 6 +++++- beacon_node/network/src/service.rs | 5 ++++- consensus/types/src/fork_context.rs | 19 ++++++++++++++++--- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index 5950941f70b..06aebeb4aa7 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -194,11 +194,15 @@ impl Network { // set up a collection of variables accessible outside of the network crate // Create an ENR or load from disk if appropriate + let next_fork_digest = ctx + .fork_context + .next_fork_digest() + .unwrap_or_else(|| ctx.fork_context.current_fork_digest()); let enr = crate::discovery::enr::build_or_load_enr::( local_keypair.clone(), &config, &ctx.enr_fork_id, - ctx.fork_context.next_fork_digest(), + next_fork_digest, &ctx.chain_spec, )?; diff --git a/beacon_node/network/src/service.rs b/beacon_node/network/src/service.rs index 1bd4f9ee398..325feda0d48 100644 --- a/beacon_node/network/src/service.rs +++ b/beacon_node/network/src/service.rs @@ -843,7 +843,10 @@ impl NetworkService { fork_context.update_current_fork(*new_fork_name, new_fork_digest, current_epoch); if self.beacon_chain.spec.is_peer_das_scheduled() { - self.libp2p.update_nfd(fork_context.next_fork_digest()); + let next_fork_digest = fork_context + .next_fork_digest() + .unwrap_or_else(|| fork_context.current_fork_digest()); + self.libp2p.update_nfd(next_fork_digest); } self.libp2p.update_fork_version(new_enr_fork_id); diff --git a/consensus/types/src/fork_context.rs b/consensus/types/src/fork_context.rs index b537e56f80e..aeb14934f49 100644 --- a/consensus/types/src/fork_context.rs +++ b/consensus/types/src/fork_context.rs @@ -3,6 +3,15 @@ use parking_lot::RwLock; use crate::{ChainSpec, Epoch, EthSpec, ForkName, Hash256, Slot}; use std::collections::BTreeMap; +/// Represents a hard fork in the consensus protocol. +/// +/// A hard fork can be one of two types: +/// * A named fork (represented by `ForkName`) which introduces protocol changes. +/// * A blob-parameter-only (BPO) fork which only modifies blob parameters. +/// +/// For BPO forks, the `fork_name` remains unchanged from the previous fork, +/// but the `fork_epoch` and `fork_digest` will be different to reflect the +/// new blob parameter changes. #[derive(Debug, Clone)] pub struct HardFork { fork_name: ForkName, @@ -77,14 +86,18 @@ impl ForkContext { self.current_fork.read().fork_epoch } + /// Returns the current fork digest. + pub fn current_fork_digest(&self) -> [u8; 4] { + self.current_fork.read().fork_digest + } + /// Returns the next fork digest. If there's no future fork, returns the current fork digest. - pub fn next_fork_digest(&self) -> [u8; 4] { + pub fn next_fork_digest(&self) -> Option<[u8; 4]> { let current_fork_epoch = self.current_fork_epoch(); self.epoch_to_forks .range(current_fork_epoch..) .nth(1) .map(|(_, fork)| fork.fork_digest) - .unwrap_or_else(|| self.current_fork.read().fork_digest) } /// Updates the `digest_epoch` field to a new digest epoch. @@ -205,7 +218,7 @@ mod tests { let context = ForkContext::new::(electra_slot, genesis_root, &spec); - let next_digest = context.next_fork_digest(); + let next_digest = context.next_fork_digest().unwrap(); let expected_digest = spec.compute_fork_digest(genesis_root, spec.fulu_fork_epoch.unwrap()); assert_eq!(next_digest, expected_digest); } From 555fb619fbd8f5402ff83d0395f18ac43809b257 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Thu, 10 Jul 2025 10:25:11 -0700 Subject: [PATCH 10/10] make sure all forks are covered in tests --- beacon_node/lighthouse_network/src/rpc/codec.rs | 3 +++ beacon_node/lighthouse_network/tests/common.rs | 3 +++ consensus/types/src/signed_beacon_block.rs | 3 +++ 3 files changed, 9 insertions(+) diff --git a/beacon_node/lighthouse_network/src/rpc/codec.rs b/beacon_node/lighthouse_network/src/rpc/codec.rs index 186f61b1e96..d01b3b76ca1 100644 --- a/beacon_node/lighthouse_network/src/rpc/codec.rs +++ b/beacon_node/lighthouse_network/src/rpc/codec.rs @@ -918,6 +918,9 @@ mod tests { chain_spec.deneb_fork_epoch = Some(Epoch::new(4)); chain_spec.electra_fork_epoch = Some(Epoch::new(5)); chain_spec.fulu_fork_epoch = Some(Epoch::new(6)); + + // check that we have all forks covered + assert!(chain_spec.fork_epoch(ForkName::latest()).is_some()); chain_spec } diff --git a/beacon_node/lighthouse_network/tests/common.rs b/beacon_node/lighthouse_network/tests/common.rs index eb945b61ce2..61f48a9a6fd 100644 --- a/beacon_node/lighthouse_network/tests/common.rs +++ b/beacon_node/lighthouse_network/tests/common.rs @@ -28,6 +28,9 @@ pub fn spec_with_all_forks_enabled() -> ChainSpec { chain_spec.deneb_fork_epoch = Some(Epoch::new(4)); chain_spec.electra_fork_epoch = Some(Epoch::new(5)); chain_spec.fulu_fork_epoch = Some(Epoch::new(6)); + + // check that we have all forks covered + assert!(chain_spec.fork_epoch(ForkName::latest()).is_some()); chain_spec } diff --git a/consensus/types/src/signed_beacon_block.rs b/consensus/types/src/signed_beacon_block.rs index 1b5ab365712..64dce93aefb 100644 --- a/consensus/types/src/signed_beacon_block.rs +++ b/consensus/types/src/signed_beacon_block.rs @@ -891,6 +891,9 @@ mod test { chain_spec.deneb_fork_epoch = Some(Epoch::new(4)); chain_spec.electra_fork_epoch = Some(Epoch::new(5)); chain_spec.fulu_fork_epoch = Some(Epoch::new(6)); + + // check that we have all forks covered + assert!(chain_spec.fork_epoch(ForkName::latest()).is_some()); chain_spec }