From 8d7b3ddac71c639c7b245149af91fc0349a5954f Mon Sep 17 00:00:00 2001 From: Age Manning Date: Wed, 30 Oct 2024 16:31:28 +1100 Subject: [PATCH 1/6] Correct gossipsub mesh and connected peer inconsistencies (#6244) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Handle gossipsub promises gracefully * Apply a forgotten patch which sync the fanout with unsubscriptions * Merge remote-tracking branch 'network/unstable' into supress-invalid-gossipsub-error * Update beacon_node/lighthouse_network/gossipsub/src/behaviour.rs Co-authored-by: João Oliveira * Add changelog entry * Merge latest unstable * Merge branch 'unstable' into supress-invalid-gossipsub-error * Merge branch 'unstable' into supress-invalid-gossipsub-error --- .../lighthouse_network/gossipsub/CHANGELOG.md | 3 ++ .../gossipsub/src/behaviour.rs | 38 +++++++++++-------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/beacon_node/lighthouse_network/gossipsub/CHANGELOG.md b/beacon_node/lighthouse_network/gossipsub/CHANGELOG.md index 006eb20a70..aba85f6184 100644 --- a/beacon_node/lighthouse_network/gossipsub/CHANGELOG.md +++ b/beacon_node/lighthouse_network/gossipsub/CHANGELOG.md @@ -2,6 +2,9 @@ - Remove the beta tag from the v1.2 upgrade. See [PR 6344](https://github.com/sigp/lighthouse/pull/6344) +- Correct state inconsistencies with the mesh and connected peers due to the fanout mapping. + See [PR 6244](https://github.com/sigp/lighthouse/pull/6244) + - Implement IDONTWANT messages as per [spec](https://github.com/libp2p/specs/pull/548). See [PR 5422](https://github.com/sigp/lighthouse/pull/5422) diff --git a/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs b/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs index c50e76e7f2..60f3d48d06 100644 --- a/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs +++ b/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs @@ -764,7 +764,7 @@ where } } else { tracing::error!(peer_id = %peer_id, - "Could not PUBLISH, peer doesn't exist in connected peer list"); + "Could not send PUBLISH, peer doesn't exist in connected peer list"); } } @@ -1066,7 +1066,7 @@ where }); } else { tracing::error!(peer = %peer_id, - "Could not GRAFT, peer doesn't exist in connected peer list"); + "Could not send GRAFT, peer doesn't exist in connected peer list"); } // If the peer did not previously exist in any mesh, inform the handler @@ -1165,7 +1165,7 @@ where peer.sender.prune(prune); } else { tracing::error!(peer = %peer_id, - "Could not PRUNE, peer doesn't exist in connected peer list"); + "Could not send PRUNE, peer doesn't exist in connected peer list"); } // If the peer did not previously exist in any mesh, inform the handler @@ -1344,7 +1344,7 @@ where } } else { tracing::error!(peer = %peer_id, - "Could not IWANT, peer doesn't exist in connected peer list"); + "Could not send IWANT, peer doesn't exist in connected peer list"); } } tracing::trace!(peer=%peer_id, "Completed IHAVE handling for peer"); @@ -1367,7 +1367,7 @@ where for id in iwant_msgs { // If we have it and the IHAVE count is not above the threshold, - // foward the message. + // forward the message. if let Some((msg, count)) = self .mcache .get_with_iwant_counts(&id, peer_id) @@ -1407,7 +1407,7 @@ where } } else { tracing::error!(peer = %peer_id, - "Could not IWANT, peer doesn't exist in connected peer list"); + "Could not send IWANT, peer doesn't exist in connected peer list"); } } } @@ -2050,8 +2050,11 @@ where } } - // remove unsubscribed peers from the mesh if it exists + // remove unsubscribed peers from the mesh and fanout if they exist there. for (peer_id, topic_hash) in unsubscribed_peers { + self.fanout + .get_mut(&topic_hash) + .map(|peers| peers.remove(&peer_id)); self.remove_peer_from_mesh(&peer_id, &topic_hash, None, false, Churn::Unsub); } @@ -2075,7 +2078,7 @@ where } } else { tracing::error!(peer = %propagation_source, - "Could not GRAFT, peer doesn't exist in connected peer list"); + "Could not send GRAFT, peer doesn't exist in connected peer list"); } // Notify the application of the subscriptions @@ -2093,9 +2096,12 @@ where fn apply_iwant_penalties(&mut self) { if let Some((peer_score, ..)) = &mut self.peer_score { for (peer, count) in self.gossip_promises.get_broken_promises() { - peer_score.add_penalty(&peer, count); - if let Some(metrics) = self.metrics.as_mut() { - metrics.register_score_penalty(Penalty::BrokenPromise); + // We do not apply penalties to nodes that have disconnected. + if self.connected_peers.contains_key(&peer) { + peer_score.add_penalty(&peer, count); + if let Some(metrics) = self.metrics.as_mut() { + metrics.register_score_penalty(Penalty::BrokenPromise); + } } } } @@ -2590,7 +2596,7 @@ where } } else { tracing::error!(peer = %peer_id, - "Could not IHAVE, peer doesn't exist in connected peer list"); + "Could not send IHAVE, peer doesn't exist in connected peer list"); } } } @@ -2676,7 +2682,7 @@ where peer.sender.prune(prune); } else { tracing::error!(peer = %peer_id, - "Could not PRUNE, peer doesn't exist in connected peer list"); + "Could not send PRUNE, peer doesn't exist in connected peer list"); } // inform the handler @@ -2713,8 +2719,8 @@ where for peer_id in recipient_peers { let Some(peer) = self.connected_peers.get_mut(peer_id) else { - tracing::error!(peer = %peer_id, - "Could not IDONTWANT, peer doesn't exist in connected peer list"); + // It can be the case that promises to disconnected peers appear here. In this case + // we simply ignore the peer-id. continue; }; @@ -2979,7 +2985,7 @@ where } } else { tracing::error!(peer = %peer_id, - "Could not SUBSCRIBE, peer doesn't exist in connected peer list"); + "Could not send SUBSCRIBE, peer doesn't exist in connected peer list"); } } From 7105442840f6702a82fe941636ea1f07737be166 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Wed, 30 Oct 2024 11:26:26 +0000 Subject: [PATCH 2/6] Remove manual poll of the libp2p Swarm (#6550) * remove manual poll for libp2p Swarm, use tokio::select! instead --- .../lighthouse_network/src/service/mod.rs | 271 +++++++++--------- 1 file changed, 134 insertions(+), 137 deletions(-) diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index f3fbd25a90..b23e417adb 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -37,10 +37,7 @@ use slog::{crit, debug, info, o, trace, warn}; use std::num::{NonZeroU8, NonZeroUsize}; use std::path::PathBuf; use std::pin::Pin; -use std::{ - sync::Arc, - task::{Context, Poll}, -}; +use std::sync::Arc; use types::{ consts::altair::SYNC_COMMITTEE_SUBNET_COUNT, EnrForkId, EthSpec, ForkContext, Slot, SubnetId, }; @@ -1794,148 +1791,148 @@ impl Network { /* Networking polling */ - /// Poll the p2p networking stack. - /// - /// This will poll the swarm and do maintenance routines. - pub fn poll_network(&mut self, cx: &mut Context) -> Poll> { - while let Poll::Ready(Some(swarm_event)) = self.swarm.poll_next_unpin(cx) { - let maybe_event = match swarm_event { - SwarmEvent::Behaviour(behaviour_event) => match behaviour_event { - // Handle sub-behaviour events. - BehaviourEvent::Gossipsub(ge) => self.inject_gs_event(ge), - BehaviourEvent::Eth2Rpc(re) => self.inject_rpc_event(re), - // Inform the peer manager about discovered peers. - // - // The peer manager will subsequently decide which peers need to be dialed and then dial - // them. - BehaviourEvent::Discovery(DiscoveredPeers { peers }) => { - self.peer_manager_mut().peers_discovered(peers); - None + pub async fn next_event(&mut self) -> NetworkEvent { + loop { + tokio::select! { + // Poll the libp2p `Swarm`. + // This will poll the swarm and do maintenance routines. + Some(event) = self.swarm.next() => { + if let Some(event) = self.parse_swarm_event(event) { + return event; } - BehaviourEvent::Identify(ie) => self.inject_identify_event(ie), - BehaviourEvent::PeerManager(pe) => self.inject_pm_event(pe), - BehaviourEvent::Upnp(e) => { - self.inject_upnp_event(e); - None - } - #[allow(unreachable_patterns)] - BehaviourEvent::ConnectionLimits(le) => void::unreachable(le), }, - SwarmEvent::ConnectionEstablished { .. } => None, - SwarmEvent::ConnectionClosed { .. } => None, - SwarmEvent::IncomingConnection { - local_addr, - send_back_addr, - connection_id: _, - } => { - trace!(self.log, "Incoming connection"; "our_addr" => %local_addr, "from" => %send_back_addr); - None + + // perform gossipsub score updates when necessary + _ = self.update_gossipsub_scores.tick() => { + let this = self.swarm.behaviour_mut(); + this.peer_manager.update_gossipsub_scores(&this.gossipsub); } - SwarmEvent::IncomingConnectionError { - local_addr, - send_back_addr, - error, - connection_id: _, - } => { - let error_repr = match error { - libp2p::swarm::ListenError::Aborted => { - "Incoming connection aborted".to_string() + // poll the gossipsub cache to clear expired messages + Some(result) = self.gossip_cache.next() => { + match result { + Err(e) => warn!(self.log, "Gossip cache error"; "error" => e), + Ok(expired_topic) => { + if let Some(v) = metrics::get_int_counter( + &metrics::GOSSIP_EXPIRED_LATE_PUBLISH_PER_TOPIC_KIND, + &[expired_topic.kind().as_ref()], + ) { + v.inc() + }; } - libp2p::swarm::ListenError::WrongPeerId { obtained, endpoint } => { - format!("Wrong peer id, obtained {obtained}, endpoint {endpoint:?}") - } - libp2p::swarm::ListenError::LocalPeerId { endpoint } => { - format!("Dialing local peer id {endpoint:?}") - } - libp2p::swarm::ListenError::Denied { cause } => { - format!("Connection was denied with cause: {cause:?}") - } - libp2p::swarm::ListenError::Transport(t) => match t { - libp2p::TransportError::MultiaddrNotSupported(m) => { - format!("Transport error: Multiaddr not supported: {m}") - } - libp2p::TransportError::Other(e) => { - format!("Transport error: other: {e}") - } - }, - }; - debug!(self.log, "Failed incoming connection"; "our_addr" => %local_addr, "from" => %send_back_addr, "error" => error_repr); - None - } - SwarmEvent::OutgoingConnectionError { - peer_id: _, - error: _, - connection_id: _, - } => { - // The Behaviour event is more general than the swarm event here. It includes - // connection failures. So we use that log for now, in the peer manager - // behaviour implementation. - None - } - SwarmEvent::NewListenAddr { address, .. } => { - Some(NetworkEvent::NewListenAddr(address)) - } - SwarmEvent::ExpiredListenAddr { address, .. } => { - debug!(self.log, "Listen address expired"; "address" => %address); - None - } - SwarmEvent::ListenerClosed { - addresses, reason, .. - } => { - match reason { - Ok(_) => { - debug!(self.log, "Listener gracefully closed"; "addresses" => ?addresses) - } - Err(reason) => { - crit!(self.log, "Listener abruptly closed"; "addresses" => ?addresses, "reason" => ?reason) - } - }; - if Swarm::listeners(&self.swarm).count() == 0 { - Some(NetworkEvent::ZeroListeners) - } else { - None } } - SwarmEvent::ListenerError { error, .. } => { - debug!(self.log, "Listener closed connection attempt"; "reason" => ?error); - None - } - _ => { - // NOTE: SwarmEvent is a non exhaustive enum so updates should be based on - // release notes more than compiler feedback - None - } - }; - - if let Some(ev) = maybe_event { - return Poll::Ready(ev); } } - - // perform gossipsub score updates when necessary - while self.update_gossipsub_scores.poll_tick(cx).is_ready() { - let this = self.swarm.behaviour_mut(); - this.peer_manager.update_gossipsub_scores(&this.gossipsub); - } - - // poll the gossipsub cache to clear expired messages - while let Poll::Ready(Some(result)) = self.gossip_cache.poll_next_unpin(cx) { - match result { - Err(e) => warn!(self.log, "Gossip cache error"; "error" => e), - Ok(expired_topic) => { - if let Some(v) = metrics::get_int_counter( - &metrics::GOSSIP_EXPIRED_LATE_PUBLISH_PER_TOPIC_KIND, - &[expired_topic.kind().as_ref()], - ) { - v.inc() - }; - } - } - } - Poll::Pending } - pub async fn next_event(&mut self) -> NetworkEvent { - futures::future::poll_fn(|cx| self.poll_network(cx)).await + fn parse_swarm_event( + &mut self, + event: SwarmEvent>, + ) -> Option> { + match event { + SwarmEvent::Behaviour(behaviour_event) => match behaviour_event { + // Handle sub-behaviour events. + BehaviourEvent::Gossipsub(ge) => self.inject_gs_event(ge), + BehaviourEvent::Eth2Rpc(re) => self.inject_rpc_event(re), + // Inform the peer manager about discovered peers. + // + // The peer manager will subsequently decide which peers need to be dialed and then dial + // them. + BehaviourEvent::Discovery(DiscoveredPeers { peers }) => { + self.peer_manager_mut().peers_discovered(peers); + None + } + BehaviourEvent::Identify(ie) => self.inject_identify_event(ie), + BehaviourEvent::PeerManager(pe) => self.inject_pm_event(pe), + BehaviourEvent::Upnp(e) => { + self.inject_upnp_event(e); + None + } + #[allow(unreachable_patterns)] + BehaviourEvent::ConnectionLimits(le) => void::unreachable(le), + }, + SwarmEvent::ConnectionEstablished { .. } => None, + SwarmEvent::ConnectionClosed { .. } => None, + SwarmEvent::IncomingConnection { + local_addr, + send_back_addr, + connection_id: _, + } => { + trace!(self.log, "Incoming connection"; "our_addr" => %local_addr, "from" => %send_back_addr); + None + } + SwarmEvent::IncomingConnectionError { + local_addr, + send_back_addr, + error, + connection_id: _, + } => { + let error_repr = match error { + libp2p::swarm::ListenError::Aborted => { + "Incoming connection aborted".to_string() + } + libp2p::swarm::ListenError::WrongPeerId { obtained, endpoint } => { + format!("Wrong peer id, obtained {obtained}, endpoint {endpoint:?}") + } + libp2p::swarm::ListenError::LocalPeerId { endpoint } => { + format!("Dialing local peer id {endpoint:?}") + } + libp2p::swarm::ListenError::Denied { cause } => { + format!("Connection was denied with cause: {cause:?}") + } + libp2p::swarm::ListenError::Transport(t) => match t { + libp2p::TransportError::MultiaddrNotSupported(m) => { + format!("Transport error: Multiaddr not supported: {m}") + } + libp2p::TransportError::Other(e) => { + format!("Transport error: other: {e}") + } + }, + }; + debug!(self.log, "Failed incoming connection"; "our_addr" => %local_addr, "from" => %send_back_addr, "error" => error_repr); + None + } + SwarmEvent::OutgoingConnectionError { + peer_id: _, + error: _, + connection_id: _, + } => { + // The Behaviour event is more general than the swarm event here. It includes + // connection failures. So we use that log for now, in the peer manager + // behaviour implementation. + None + } + SwarmEvent::NewListenAddr { address, .. } => Some(NetworkEvent::NewListenAddr(address)), + SwarmEvent::ExpiredListenAddr { address, .. } => { + debug!(self.log, "Listen address expired"; "address" => %address); + None + } + SwarmEvent::ListenerClosed { + addresses, reason, .. + } => { + match reason { + Ok(_) => { + debug!(self.log, "Listener gracefully closed"; "addresses" => ?addresses) + } + Err(reason) => { + crit!(self.log, "Listener abruptly closed"; "addresses" => ?addresses, "reason" => ?reason) + } + }; + if Swarm::listeners(&self.swarm).count() == 0 { + Some(NetworkEvent::ZeroListeners) + } else { + None + } + } + SwarmEvent::ListenerError { error, .. } => { + debug!(self.log, "Listener closed connection attempt"; "reason" => ?error); + None + } + _ => { + // NOTE: SwarmEvent is a non exhaustive enum so updates should be based on + // release notes more than compiler feedback + None + } + } } } From 11260585d7944af42be7af90ee4d5f8c121bd679 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 31 Oct 2024 18:35:53 +1100 Subject: [PATCH 3/6] Pin `kurtosis-cli` version (#6555) * Test old version of kurtosis. --- .github/workflows/local-testnet.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/local-testnet.yml b/.github/workflows/local-testnet.yml index bcade948d7..f719360c6a 100644 --- a/.github/workflows/local-testnet.yml +++ b/.github/workflows/local-testnet.yml @@ -41,7 +41,7 @@ jobs: sudo add-apt-repository ppa:rmescandon/yq echo "deb [trusted=yes] https://apt.fury.io/kurtosis-tech/ /" | sudo tee /etc/apt/sources.list.d/kurtosis.list sudo apt update - sudo apt install -y kurtosis-cli yq + sudo apt install -y kurtosis-cli=1.3.1 yq kurtosis analytics disable - name: Download Docker image artifact @@ -88,7 +88,7 @@ jobs: sudo add-apt-repository ppa:rmescandon/yq echo "deb [trusted=yes] https://apt.fury.io/kurtosis-tech/ /" | sudo tee /etc/apt/sources.list.d/kurtosis.list sudo apt update - sudo apt install -y kurtosis-cli yq + sudo apt install -y kurtosis-cli=1.3.1 yq kurtosis analytics disable - name: Download Docker image artifact @@ -124,7 +124,7 @@ jobs: sudo add-apt-repository ppa:rmescandon/yq echo "deb [trusted=yes] https://apt.fury.io/kurtosis-tech/ /" | sudo tee /etc/apt/sources.list.d/kurtosis.list sudo apt update - sudo apt install -y kurtosis-cli yq + sudo apt install -y kurtosis-cli=1.3.1 yq kurtosis analytics disable - name: Download Docker image artifact From 16693b0bd75fc897edfe369b3fdc1cbe5a651755 Mon Sep 17 00:00:00 2001 From: zhiqiangxu <652732310@qq.com> Date: Fri, 1 Nov 2024 14:06:53 +0800 Subject: [PATCH 4/6] make `execution-endpoint` required (#5165) * make `execution-endpoint` mandatory * use parse_required instead * make test pass * Merge branch 'unstable' into make_ee_required * fix test * Merge branch 'unstable' into make_ee_required * Fix cli help text * Fix tests * Merge branch 'unstable' into make_ee_required * Add comment * Clarification * Merge remote-tracking branch 'origin/unstable' into make_ee_required --- beacon_node/src/cli.rs | 1 + beacon_node/src/config.rs | 162 ++++++++++++++++---------------- book/src/help_bn.md | 2 +- lighthouse/tests/beacon_node.rs | 40 +++++--- 4 files changed, 111 insertions(+), 94 deletions(-) diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index dff030fb0f..34b03a0955 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -793,6 +793,7 @@ pub fn cli_app() -> Command { .help("Server endpoint for an execution layer JWT-authenticated HTTP \ JSON-RPC connection. Uses the same endpoint to populate the \ deposit cache.") + .required(true) .action(ArgAction::Set) .display_order(0) ) diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 2d31815351..ecadee5f47 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -284,93 +284,95 @@ pub fn get_config( client_config.eth1.cache_follow_distance = Some(follow_distance); } - if let Some(endpoints) = cli_args.get_one::("execution-endpoint") { - let mut el_config = execution_layer::Config::default(); + // `--execution-endpoint` is required now. + let endpoints: String = clap_utils::parse_required(cli_args, "execution-endpoint")?; + let mut el_config = execution_layer::Config::default(); - // Always follow the deposit contract when there is an execution endpoint. - // - // This is wasteful for non-staking nodes as they have no need to process deposit contract - // logs and build an "eth1" cache. The alternative is to explicitly require the `--eth1` or - // `--staking` flags, however that poses a risk to stakers since they cannot produce blocks - // without "eth1". - // - // The waste for non-staking nodes is relatively small so we err on the side of safety for - // stakers. The merge is already complicated enough. - client_config.sync_eth1_chain = true; + // Always follow the deposit contract when there is an execution endpoint. + // + // This is wasteful for non-staking nodes as they have no need to process deposit contract + // logs and build an "eth1" cache. The alternative is to explicitly require the `--eth1` or + // `--staking` flags, however that poses a risk to stakers since they cannot produce blocks + // without "eth1". + // + // The waste for non-staking nodes is relatively small so we err on the side of safety for + // stakers. The merge is already complicated enough. + client_config.sync_eth1_chain = true; - // Parse a single execution endpoint, logging warnings if multiple endpoints are supplied. - let execution_endpoint = - parse_only_one_value(endpoints, SensitiveUrl::parse, "--execution-endpoint", log)?; + // Parse a single execution endpoint, logging warnings if multiple endpoints are supplied. + let execution_endpoint = parse_only_one_value( + endpoints.as_str(), + SensitiveUrl::parse, + "--execution-endpoint", + log, + )?; - // JWTs are required if `--execution-endpoint` is supplied. They can be either passed via - // file_path or directly as string. + // JWTs are required if `--execution-endpoint` is supplied. They can be either passed via + // file_path or directly as string. - let secret_file: PathBuf; - // Parse a single JWT secret from a given file_path, logging warnings if multiple are supplied. - if let Some(secret_files) = cli_args.get_one::("execution-jwt") { - secret_file = - parse_only_one_value(secret_files, PathBuf::from_str, "--execution-jwt", log)?; + let secret_file: PathBuf; + // Parse a single JWT secret from a given file_path, logging warnings if multiple are supplied. + if let Some(secret_files) = cli_args.get_one::("execution-jwt") { + secret_file = + parse_only_one_value(secret_files, PathBuf::from_str, "--execution-jwt", log)?; - // Check if the JWT secret key is passed directly via cli flag and persist it to the default - // file location. - } else if let Some(jwt_secret_key) = cli_args.get_one::("execution-jwt-secret-key") - { - use std::fs::File; - use std::io::Write; - secret_file = client_config.data_dir().join(DEFAULT_JWT_FILE); - let mut jwt_secret_key_file = File::create(secret_file.clone()) - .map_err(|e| format!("Error while creating jwt_secret_key file: {:?}", e))?; - jwt_secret_key_file - .write_all(jwt_secret_key.as_bytes()) - .map_err(|e| { - format!( - "Error occurred while writing to jwt_secret_key file: {:?}", - e - ) - })?; - } else { - return Err("Error! Please set either --execution-jwt file_path or --execution-jwt-secret-key directly via cli when using --execution-endpoint".to_string()); - } - - // Parse and set the payload builder, if any. - if let Some(endpoint) = cli_args.get_one::("builder") { - let payload_builder = - parse_only_one_value(endpoint, SensitiveUrl::parse, "--builder", log)?; - el_config.builder_url = Some(payload_builder); - - el_config.builder_user_agent = - clap_utils::parse_optional(cli_args, "builder-user-agent")?; - - el_config.builder_header_timeout = - clap_utils::parse_optional(cli_args, "builder-header-timeout")? - .map(Duration::from_millis); - } - - // Set config values from parse values. - el_config.secret_file = Some(secret_file.clone()); - el_config.execution_endpoint = Some(execution_endpoint.clone()); - el_config.suggested_fee_recipient = - clap_utils::parse_optional(cli_args, "suggested-fee-recipient")?; - el_config.jwt_id = clap_utils::parse_optional(cli_args, "execution-jwt-id")?; - el_config.jwt_version = clap_utils::parse_optional(cli_args, "execution-jwt-version")?; - el_config - .default_datadir - .clone_from(client_config.data_dir()); - let execution_timeout_multiplier = - clap_utils::parse_required(cli_args, "execution-timeout-multiplier")?; - el_config.execution_timeout_multiplier = Some(execution_timeout_multiplier); - - client_config.eth1.endpoint = Eth1Endpoint::Auth { - endpoint: execution_endpoint, - jwt_path: secret_file, - jwt_id: el_config.jwt_id.clone(), - jwt_version: el_config.jwt_version.clone(), - }; - - // Store the EL config in the client config. - client_config.execution_layer = Some(el_config); + // Check if the JWT secret key is passed directly via cli flag and persist it to the default + // file location. + } else if let Some(jwt_secret_key) = cli_args.get_one::("execution-jwt-secret-key") { + use std::fs::File; + use std::io::Write; + secret_file = client_config.data_dir().join(DEFAULT_JWT_FILE); + let mut jwt_secret_key_file = File::create(secret_file.clone()) + .map_err(|e| format!("Error while creating jwt_secret_key file: {:?}", e))?; + jwt_secret_key_file + .write_all(jwt_secret_key.as_bytes()) + .map_err(|e| { + format!( + "Error occurred while writing to jwt_secret_key file: {:?}", + e + ) + })?; + } else { + return Err("Error! Please set either --execution-jwt file_path or --execution-jwt-secret-key directly via cli when using --execution-endpoint".to_string()); } + // Parse and set the payload builder, if any. + if let Some(endpoint) = cli_args.get_one::("builder") { + let payload_builder = + parse_only_one_value(endpoint, SensitiveUrl::parse, "--builder", log)?; + el_config.builder_url = Some(payload_builder); + + el_config.builder_user_agent = clap_utils::parse_optional(cli_args, "builder-user-agent")?; + + el_config.builder_header_timeout = + clap_utils::parse_optional(cli_args, "builder-header-timeout")? + .map(Duration::from_millis); + } + + // Set config values from parse values. + el_config.secret_file = Some(secret_file.clone()); + el_config.execution_endpoint = Some(execution_endpoint.clone()); + el_config.suggested_fee_recipient = + clap_utils::parse_optional(cli_args, "suggested-fee-recipient")?; + el_config.jwt_id = clap_utils::parse_optional(cli_args, "execution-jwt-id")?; + el_config.jwt_version = clap_utils::parse_optional(cli_args, "execution-jwt-version")?; + el_config + .default_datadir + .clone_from(client_config.data_dir()); + let execution_timeout_multiplier = + clap_utils::parse_required(cli_args, "execution-timeout-multiplier")?; + el_config.execution_timeout_multiplier = Some(execution_timeout_multiplier); + + client_config.eth1.endpoint = Eth1Endpoint::Auth { + endpoint: execution_endpoint, + jwt_path: secret_file, + jwt_id: el_config.jwt_id.clone(), + jwt_version: el_config.jwt_version.clone(), + }; + + // Store the EL config in the client config. + client_config.execution_layer = Some(el_config); + // 4844 params if let Some(trusted_setup) = context .eth2_network_config diff --git a/book/src/help_bn.md b/book/src/help_bn.md index 69701a3ad9..fa4a473ec0 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -5,7 +5,7 @@ The primary component which connects to the Ethereum 2.0 P2P network and downloads, verifies and stores blocks. Provides a HTTP API for querying the beacon chain and publishing messages to the network. -Usage: lighthouse beacon_node [OPTIONS] +Usage: lighthouse beacon_node [OPTIONS] --execution-endpoint Options: --auto-compact-db diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index ac7ddcdbd9..ffa6e300a7 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -24,7 +24,9 @@ use types::non_zero_usize::new_non_zero_usize; use types::{Address, Checkpoint, Epoch, Hash256, MainnetEthSpec}; use unused_port::{unused_tcp4_port, unused_tcp6_port, unused_udp4_port, unused_udp6_port}; -const DEFAULT_ETH1_ENDPOINT: &str = "http://localhost:8545/"; +const DEFAULT_EXECUTION_ENDPOINT: &str = "http://localhost:8551/"; +const DEFAULT_EXECUTION_JWT_SECRET_KEY: &str = + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; // These dummy ports should ONLY be used for `enr-xxx-port` flags that do not bind. const DUMMY_ENR_TCP_PORT: u16 = 7777; @@ -52,6 +54,18 @@ struct CommandLineTest { } impl CommandLineTest { fn new() -> CommandLineTest { + let mut base_cmd = base_cmd(); + + base_cmd + .arg("--execution-endpoint") + .arg(DEFAULT_EXECUTION_ENDPOINT) + .arg("--execution-jwt-secret-key") + .arg(DEFAULT_EXECUTION_JWT_SECRET_KEY); + CommandLineTest { cmd: base_cmd } + } + + // Required for testing different JWT authentication methods. + fn new_with_no_execution_endpoint() -> CommandLineTest { let base_cmd = base_cmd(); CommandLineTest { cmd: base_cmd } } @@ -104,7 +118,7 @@ fn staking_flag() { assert!(config.sync_eth1_chain); assert_eq!( config.eth1.endpoint.get_endpoint().to_string(), - DEFAULT_ETH1_ENDPOINT + DEFAULT_EXECUTION_ENDPOINT ); }); } @@ -253,7 +267,7 @@ fn always_prepare_payload_default() { #[test] fn always_prepare_payload_override() { let dir = TempDir::new().expect("Unable to create temporary directory"); - CommandLineTest::new() + CommandLineTest::new_with_no_execution_endpoint() .flag("always-prepare-payload", None) .flag( "suggested-fee-recipient", @@ -459,7 +473,7 @@ fn run_bellatrix_execution_endpoints_flag_test(flag: &str) { // this is way better but intersperse is still a nightly feature :/ // let endpoint_arg: String = urls.into_iter().intersperse(",").collect(); - CommandLineTest::new() + CommandLineTest::new_with_no_execution_endpoint() .flag(flag, Some(&endpoint_arg)) .flag("execution-jwt", Some(&jwts_arg)) .run_with_zero_port() @@ -480,7 +494,7 @@ fn run_bellatrix_execution_endpoints_flag_test(flag: &str) { #[test] fn run_execution_jwt_secret_key_is_persisted() { let jwt_secret_key = "0x3cbc11b0d8fa16f3344eacfd6ff6430b9d30734450e8adcf5400f88d327dcb33"; - CommandLineTest::new() + CommandLineTest::new_with_no_execution_endpoint() .flag("execution-endpoint", Some("http://localhost:8551/")) .flag("execution-jwt-secret-key", Some(jwt_secret_key)) .run_with_zero_port() @@ -501,7 +515,7 @@ fn run_execution_jwt_secret_key_is_persisted() { #[test] fn execution_timeout_multiplier_flag() { let dir = TempDir::new().expect("Unable to create temporary directory"); - CommandLineTest::new() + CommandLineTest::new_with_no_execution_endpoint() .flag("execution-endpoint", Some("http://meow.cats")) .flag( "execution-jwt", @@ -528,7 +542,7 @@ fn bellatrix_jwt_secrets_flag() { let mut file = File::create(dir.path().join("jwtsecrets")).expect("Unable to create file"); file.write_all(b"0x3cbc11b0d8fa16f3344eacfd6ff6430b9d30734450e8adcf5400f88d327dcb33") .expect("Unable to write to file"); - CommandLineTest::new() + CommandLineTest::new_with_no_execution_endpoint() .flag("execution-endpoints", Some("http://localhost:8551/")) .flag( "jwt-secrets", @@ -550,7 +564,7 @@ fn bellatrix_jwt_secrets_flag() { #[test] fn bellatrix_fee_recipient_flag() { let dir = TempDir::new().expect("Unable to create temporary directory"); - CommandLineTest::new() + CommandLineTest::new_with_no_execution_endpoint() .flag("execution-endpoint", Some("http://meow.cats")) .flag( "execution-jwt", @@ -591,7 +605,7 @@ fn run_payload_builder_flag_test_with_config( f: F, ) { let dir = TempDir::new().expect("Unable to create temporary directory"); - let mut test = CommandLineTest::new(); + let mut test = CommandLineTest::new_with_no_execution_endpoint(); test.flag("execution-endpoint", Some("http://meow.cats")) .flag( "execution-jwt", @@ -713,7 +727,7 @@ fn run_jwt_optional_flags_test(jwt_flag: &str, jwt_id_flag: &str, jwt_version_fl let jwt_file = "jwt-file"; let id = "bn-1"; let version = "Lighthouse-v2.1.3"; - CommandLineTest::new() + CommandLineTest::new_with_no_execution_endpoint() .flag("execution-endpoint", Some(execution_endpoint)) .flag(jwt_flag, dir.path().join(jwt_file).as_os_str().to_str()) .flag(jwt_id_flag, Some(id)) @@ -2430,13 +2444,13 @@ fn logfile_format_flag() { fn sync_eth1_chain_default() { CommandLineTest::new() .run_with_zero_port() - .with_config(|config| assert_eq!(config.sync_eth1_chain, false)); + .with_config(|config| assert_eq!(config.sync_eth1_chain, true)); } #[test] fn sync_eth1_chain_execution_endpoints_flag() { let dir = TempDir::new().expect("Unable to create temporary directory"); - CommandLineTest::new() + CommandLineTest::new_with_no_execution_endpoint() .flag("execution-endpoints", Some("http://localhost:8551/")) .flag( "execution-jwt", @@ -2449,7 +2463,7 @@ fn sync_eth1_chain_execution_endpoints_flag() { #[test] fn sync_eth1_chain_disable_deposit_contract_sync_flag() { let dir = TempDir::new().expect("Unable to create temporary directory"); - CommandLineTest::new() + CommandLineTest::new_with_no_execution_endpoint() .flag("disable-deposit-contract-sync", None) .flag("execution-endpoints", Some("http://localhost:8551/")) .flag( From 4f86d950e98d73b590a7dbf5225855c0bd0602cb Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Mon, 4 Nov 2024 09:46:25 +1100 Subject: [PATCH 5/6] Add error message for duration subtraction overflow in sim tests (#6558) * Add error message for duration subtraction overflow. --- testing/simulator/src/basic_sim.rs | 2 +- testing/simulator/src/fallback_sim.rs | 2 +- testing/simulator/src/local_network.rs | 7 +++++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/testing/simulator/src/basic_sim.rs b/testing/simulator/src/basic_sim.rs index e1cef95cd3..5c9baa2349 100644 --- a/testing/simulator/src/basic_sim.rs +++ b/testing/simulator/src/basic_sim.rs @@ -206,7 +206,7 @@ pub fn run_basic_sim(matches: &ArgMatches) -> Result<(), String> { node.server.all_payloads_valid(); }); - let duration_to_genesis = network.duration_to_genesis().await; + let duration_to_genesis = network.duration_to_genesis().await?; println!("Duration to genesis: {}", duration_to_genesis.as_secs()); sleep(duration_to_genesis).await; diff --git a/testing/simulator/src/fallback_sim.rs b/testing/simulator/src/fallback_sim.rs index 3859257fb7..0690ab242c 100644 --- a/testing/simulator/src/fallback_sim.rs +++ b/testing/simulator/src/fallback_sim.rs @@ -194,7 +194,7 @@ pub fn run_fallback_sim(matches: &ArgMatches) -> Result<(), String> { ); } - let duration_to_genesis = network.duration_to_genesis().await; + let duration_to_genesis = network.duration_to_genesis().await?; println!("Duration to genesis: {}", duration_to_genesis.as_secs()); sleep(duration_to_genesis).await; diff --git a/testing/simulator/src/local_network.rs b/testing/simulator/src/local_network.rs index 7b9327a7aa..59efc09baa 100644 --- a/testing/simulator/src/local_network.rs +++ b/testing/simulator/src/local_network.rs @@ -459,7 +459,7 @@ impl LocalNetwork { .map(|body| body.unwrap().data.finalized.epoch) } - pub async fn duration_to_genesis(&self) -> Duration { + pub async fn duration_to_genesis(&self) -> Result { let nodes = self.remote_nodes().expect("Failed to get remote nodes"); let bootnode = nodes.first().expect("Should contain bootnode"); let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap(); @@ -471,6 +471,9 @@ impl LocalNetwork { .data .genesis_time, ); - genesis_time - now + genesis_time.checked_sub(now).ok_or( + "The genesis time has already passed since all nodes started. The node startup time \ + may have regressed, and the current `GENESIS_DELAY` is no longer sufficient.", + ) } } From 9f657b0f07cad0829e8de8d11c44066b92526f26 Mon Sep 17 00:00:00 2001 From: Mac L Date: Tue, 5 Nov 2024 02:15:29 +0400 Subject: [PATCH 6/6] Fix doc-test in `consensus` crate (#6561) * Use correct crate name in doc-test --- consensus/types/src/runtime_var_list.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consensus/types/src/runtime_var_list.rs b/consensus/types/src/runtime_var_list.rs index af4ee87c15..8290876fa1 100644 --- a/consensus/types/src/runtime_var_list.rs +++ b/consensus/types/src/runtime_var_list.rs @@ -13,7 +13,7 @@ use std::slice::SliceIndex; /// ## Example /// /// ``` -/// use ssz_types::{RuntimeVariableList}; +/// use types::{RuntimeVariableList}; /// /// let base: Vec = vec![1, 2, 3, 4]; ///