Fix genesis state download panic when running in debug mode (sigp#4753)

## Issue Addressed sigp#4738 ## Proposed Changes See the above issue for details. Went with option #2 to use the async reqwest client in `Eth2NetworkConfig` and propagate the async-ness.
armaganyildirak · Sep 21, 2023 · a0478da · a0478da
1 parent 082bb2d
commit a0478da
Show file tree

Hide file tree

Showing 12 changed files with 90 additions and 91 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Dockerfile b/Dockerfile
@@ -1,4 +1,4 @@
-FROM rust:1.68.2-bullseye AS builder
+FROM rust:1.69.0-bullseye AS builder
 RUN apt-get update && apt-get -y upgrade && apt-get install -y cmake libclang-dev
 COPY . lighthouse
 ARG FEATURES

diff --git a/account_manager/src/validator/exit.rs b/account_manager/src/validator/exit.rs
@@ -10,7 +10,6 @@ use eth2_keystore::Keystore;
 use eth2_network_config::Eth2NetworkConfig;
 use safe_arith::SafeArith;
 use sensitive_url::SensitiveUrl;
-use slog::Logger;
 use slot_clock::{SlotClock, SystemTimeSlotClock};
 use std::path::{Path, PathBuf};
 use std::time::Duration;
@@ -79,12 +78,6 @@ pub fn cli_run<E: EthSpec>(matches: &ArgMatches, env: Environment<E>) -> Result<
     let password_file_path: Option<PathBuf> =
         clap_utils::parse_optional(matches, PASSWORD_FILE_FLAG)?;
 
-    let genesis_state_url: Option<String> =
-        clap_utils::parse_optional(matches, "genesis-state-url")?;
-    let genesis_state_url_timeout =
-        clap_utils::parse_required(matches, "genesis-state-url-timeout")
-            .map(Duration::from_secs)?;
-
     let stdin_inputs = cfg!(windows) || matches.is_present(STDIN_INPUTS_FLAG);
     let no_wait = matches.is_present(NO_WAIT);
     let no_confirmation = matches.is_present(NO_CONFIRMATION);
@@ -111,9 +104,6 @@ pub fn cli_run<E: EthSpec>(matches: &ArgMatches, env: Environment<E>) -> Result<
         &eth2_network_config,
         no_wait,
         no_confirmation,
-        genesis_state_url,
-        genesis_state_url_timeout,
-        env.core_context().log(),
     ))?;
 
     Ok(())
@@ -130,13 +120,10 @@ async fn publish_voluntary_exit<E: EthSpec>(
     eth2_network_config: &Eth2NetworkConfig,
     no_wait: bool,
     no_confirmation: bool,
-    genesis_state_url: Option<String>,
-    genesis_state_url_timeout: Duration,
-    log: &Logger,
 ) -> Result<(), String> {
     let genesis_data = get_geneisis_data(client).await?;
     let testnet_genesis_root = eth2_network_config
-        .genesis_validators_root::<E>(genesis_state_url.as_deref(), genesis_state_url_timeout, log)?
+        .genesis_validators_root::<E>()?
         .ok_or("Genesis state is unknown")?;
 
     // Verify that the beacon node and validator being exited are on the same network.

diff --git a/account_manager/src/validator/slashing_protection.rs b/account_manager/src/validator/slashing_protection.rs
@@ -7,7 +7,6 @@ use slashing_protection::{
 use std::fs::File;
 use std::path::PathBuf;
 use std::str::FromStr;
-use std::time::Duration;
 use types::{Epoch, EthSpec, PublicKeyBytes, Slot};
 
 pub const CMD: &str = "slashing-protection";
@@ -82,24 +81,12 @@ pub fn cli_run<T: EthSpec>(
     validator_base_dir: PathBuf,
 ) -> Result<(), String> {
     let slashing_protection_db_path = validator_base_dir.join(SLASHING_PROTECTION_FILENAME);
-
-    let genesis_state_url: Option<String> =
-        clap_utils::parse_optional(matches, "genesis-state-url")?;
-    let genesis_state_url_timeout =
-        clap_utils::parse_required(matches, "genesis-state-url-timeout")
-            .map(Duration::from_secs)?;
-
-    let context = env.core_context();
     let eth2_network_config = env
         .eth2_network_config
         .ok_or("Unable to get testnet configuration from the environment")?;
 
     let genesis_validators_root = eth2_network_config
-        .genesis_validators_root::<T>(
-            genesis_state_url.as_deref(),
-            genesis_state_url_timeout,
-            context.log(),
-        )?
+        .genesis_validators_root::<T>()?
         .ok_or_else(|| "Unable to get genesis state, has genesis occurred?".to_string())?;
 
     match matches.subcommand() {

diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs
@@ -256,7 +256,7 @@ where
                     "Starting from known genesis state";
                 );
 
-                let genesis_state = genesis_state(&runtime_context, &config, log)?;
+                let genesis_state = genesis_state(&runtime_context, &config, log).await?;
 
                 builder.genesis_state(genesis_state).map(|v| (v, None))?
             }
@@ -276,7 +276,7 @@ where
                     .map_err(|e| format!("Unable to parse weak subj state SSZ: {:?}", e))?;
                 let anchor_block = SignedBeaconBlock::from_ssz_bytes(&anchor_block_bytes, &spec)
                     .map_err(|e| format!("Unable to parse weak subj block SSZ: {:?}", e))?;
-                let genesis_state = genesis_state(&runtime_context, &config, log)?;
+                let genesis_state = genesis_state(&runtime_context, &config, log).await?;
 
                 builder
                     .weak_subjectivity_state(anchor_state, anchor_block, genesis_state)
@@ -377,7 +377,7 @@ where
 
                 debug!(context.log(), "Downloaded finalized block");
 
-                let genesis_state = genesis_state(&runtime_context, &config, log)?;
+                let genesis_state = genesis_state(&runtime_context, &config, log).await?;
 
                 info!(
                     context.log(),
@@ -1083,7 +1083,7 @@ where
 }
 
 /// Obtain the genesis state from the `eth2_network_config` in `context`.
-fn genesis_state<T: EthSpec>(
+async fn genesis_state<T: EthSpec>(
     context: &RuntimeContext<T>,
     config: &ClientConfig,
     log: &Logger,
@@ -1097,6 +1097,7 @@ fn genesis_state<T: EthSpec>(
             config.genesis_state_url.as_deref(),
             config.genesis_state_url_timeout,
             log,
-        )?
+        )
+        .await?
         .ok_or_else(|| "Genesis state is unknown".to_string())
 }
diff --git a/boot_node/src/config.rs b/boot_node/src/config.rs
@@ -25,7 +25,7 @@ pub struct BootNodeConfig<T: EthSpec> {
 }
 
 impl<T: EthSpec> BootNodeConfig<T> {
-    pub fn new(
+    pub async fn new(
         matches: &ArgMatches<'_>,
         eth2_network_config: &Eth2NetworkConfig,
     ) -> Result<Self, String> {
@@ -99,7 +99,7 @@ impl<T: EthSpec> BootNodeConfig<T> {
 
                 if eth2_network_config.genesis_state_is_known() {
                     let genesis_state = eth2_network_config
-                        .genesis_state::<T>(genesis_state_url.as_deref(), genesis_state_url_timeout, &logger)?
+                        .genesis_state::<T>(genesis_state_url.as_deref(), genesis_state_url_timeout, &logger).await?
                         .ok_or_else(|| {
                             "The genesis state for this network is not known, this is an unsupported mode"
                                 .to_string()

diff --git a/boot_node/src/lib.rs b/boot_node/src/lib.rs
@@ -7,7 +7,7 @@ mod cli;
 pub mod config;
 mod server;
 pub use cli::cli_app;
-use config::{BootNodeConfig, BootNodeConfigSerialization};
+use config::BootNodeConfig;
 use types::{EthSpec, EthSpecId};
 
 const LOG_CHANNEL_SIZE: usize = 2048;
@@ -81,20 +81,13 @@ fn main<T: EthSpec>(
         .build()
         .map_err(|e| format!("Failed to build runtime: {}", e))?;
 
-    // parse the CLI args into a useable config
-    let config: BootNodeConfig<T> = BootNodeConfig::new(bn_matches, eth2_network_config)?;
-
-    // Dump configs if `dump-config` or `dump-chain-config` flags are set
-    let config_sz = BootNodeConfigSerialization::from_config_ref(&config);
-    clap_utils::check_dump_configs::<_, T>(
+    // Run the boot node
+    runtime.block_on(server::run::<T>(
         lh_matches,
-        &config_sz,
-        &eth2_network_config.chain_spec::<T>()?,
-    )?;
+        bn_matches,
+        eth2_network_config,
+        log,
+    ))?;
 
-    // Run the boot node
-    if !lh_matches.is_present("immediate-shutdown") {
-        runtime.block_on(server::run(config, log));
-    }
     Ok(())
 }
diff --git a/boot_node/src/server.rs b/boot_node/src/server.rs
@@ -1,14 +1,37 @@
 //! The main bootnode server execution.
 
 use super::BootNodeConfig;
+use crate::config::BootNodeConfigSerialization;
+use clap::ArgMatches;
+use eth2_network_config::Eth2NetworkConfig;
 use lighthouse_network::{
     discv5::{enr::NodeId, Discv5, Discv5Event},
     EnrExt, Eth2Enr,
 };
 use slog::info;
 use types::EthSpec;
 
-pub async fn run<T: EthSpec>(config: BootNodeConfig<T>, log: slog::Logger) {
+pub async fn run<T: EthSpec>(
+    lh_matches: &ArgMatches<'_>,
+    bn_matches: &ArgMatches<'_>,
+    eth2_network_config: &Eth2NetworkConfig,
+    log: slog::Logger,
+) -> Result<(), String> {
+    // parse the CLI args into a useable config
+    let config: BootNodeConfig<T> = BootNodeConfig::new(bn_matches, eth2_network_config).await?;
+
+    // Dump configs if `dump-config` or `dump-chain-config` flags are set
+    let config_sz = BootNodeConfigSerialization::from_config_ref(&config);
+    clap_utils::check_dump_configs::<_, T>(
+        lh_matches,
+        &config_sz,
+        &eth2_network_config.chain_spec::<T>()?,
+    )?;
+
+    if lh_matches.is_present("immediate-shutdown") {
+        return Ok(());
+    }
+
     let BootNodeConfig {
         boot_nodes,
         local_enr,
@@ -65,8 +88,7 @@ pub async fn run<T: EthSpec>(config: BootNodeConfig<T>, log: slog::Logger) {
 
     // start the server
     if let Err(e) = discv5.start().await {
-        slog::crit!(log, "Could not start discv5 server"; "error" => %e);
-        return;
+        return Err(format!("Could not start discv5 server: {e:?}"));
     }
 
     // if there are peers in the local routing table, establish a session by running a query
@@ -82,8 +104,7 @@ pub async fn run<T: EthSpec>(config: BootNodeConfig<T>, log: slog::Logger) {
     let mut event_stream = match discv5.event_stream().await {
         Ok(stream) => stream,
         Err(e) => {
-            slog::crit!(log, "Failed to obtain event stream"; "error" => %e);
-            return;
+            return Err(format!("Failed to obtain event stream: {e:?}"));
         }
     };
 

diff --git a/common/eth2_network_config/Cargo.toml b/common/eth2_network_config/Cargo.toml
@@ -12,6 +12,7 @@ eth2_config = { path = "../eth2_config" }
 
 [dev-dependencies]
 tempfile = "3.1.0"
+tokio = "1.14.0"
 
 [dependencies]
 serde_yaml = "0.8.13"
@@ -26,3 +27,5 @@ url = "2.2.2"
 sensitive_url = { path = "../sensitive_url" }
 slog = "2.5.2"
 logging = { path = "../logging" }
+futures = "0.3.7"
+bytes = "1.1.0"