Skip to content

Commit

Permalink
Fix genesis state download panic when running in debug mode (#4753)
Browse files Browse the repository at this point in the history
## Issue Addressed

#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.
  • Loading branch information
jimmygchen committed Sep 21, 2023
1 parent 082bb2d commit a0478da
Show file tree
Hide file tree
Showing 12 changed files with 90 additions and 91 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -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
Expand Down
15 changes: 1 addition & 14 deletions account_manager/src/validator/exit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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(())
Expand All @@ -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.
Expand Down
15 changes: 1 addition & 14 deletions account_manager/src/validator/slashing_protection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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() {
Expand Down
11 changes: 6 additions & 5 deletions beacon_node/client/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))?
}
Expand All @@ -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)
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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,
Expand All @@ -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())
}
4 changes: 2 additions & 2 deletions boot_node/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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()
Expand Down
21 changes: 7 additions & 14 deletions boot_node/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(())
}
31 changes: 26 additions & 5 deletions boot_node/src/server.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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:?}"));
}
};

Expand Down
3 changes: 3 additions & 0 deletions common/eth2_network_config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Loading

0 comments on commit a0478da

Please sign in to comment.