From c591fcd20179a8fd8cb3c601d949513a193c3351 Mon Sep 17 00:00:00 2001 From: GeemoCandama Date: Fri, 11 Nov 2022 00:38:28 +0000 Subject: [PATCH] add checkpoint-sync-url-timeout flag (#3710) ## Issue Addressed #3702 Which issue # does this PR address? #3702 ## Proposed Changes Added checkpoint-sync-url-timeout flag to cli. Added timeout field to ClientGenesis::CheckpointSyncUrl to utilize timeout set ## Additional Info Please provide any additional information. For example, future considerations or information useful for reviewers. Co-authored-by: GeemoCandama <104614073+GeemoCandama@users.noreply.github.com> Co-authored-by: Michael Sproul --- beacon_node/beacon_chain/src/chain_config.rs | 3 +++ beacon_node/client/src/builder.rs | 11 ++++++----- beacon_node/src/cli.rs | 8 ++++++++ beacon_node/src/config.rs | 2 ++ lighthouse/tests/beacon_node.rs | 19 +++++++++++++++++++ 5 files changed, 38 insertions(+), 5 deletions(-) diff --git a/beacon_node/beacon_chain/src/chain_config.rs b/beacon_node/beacon_chain/src/chain_config.rs index 5e16a29cf38..286cc17a963 100644 --- a/beacon_node/beacon_chain/src/chain_config.rs +++ b/beacon_node/beacon_chain/src/chain_config.rs @@ -45,6 +45,8 @@ pub struct ChainConfig { pub paranoid_block_proposal: bool, /// Whether to strictly count unrealized justified votes. pub count_unrealized_full: CountUnrealizedFull, + /// Optionally set timeout for calls to checkpoint sync endpoint. + pub checkpoint_sync_url_timeout: u64, } impl Default for ChainConfig { @@ -65,6 +67,7 @@ impl Default for ChainConfig { always_reset_payload_statuses: false, paranoid_block_proposal: false, count_unrealized_full: CountUnrealizedFull::default(), + checkpoint_sync_url_timeout: 60, } } } diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index 36d6491a568..75b865407e2 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -40,9 +40,6 @@ use types::{ /// Interval between polling the eth1 node for genesis information. pub const ETH1_GENESIS_UPDATE_INTERVAL_MILLIS: u64 = 7_000; -/// Timeout for checkpoint sync HTTP requests. -pub const CHECKPOINT_SYNC_HTTP_TIMEOUT: Duration = Duration::from_secs(60); - /// Builds a `Client` instance. /// /// ## Notes @@ -273,8 +270,12 @@ where "remote_url" => %url, ); - let remote = - BeaconNodeHttpClient::new(url, Timeouts::set_all(CHECKPOINT_SYNC_HTTP_TIMEOUT)); + let remote = BeaconNodeHttpClient::new( + url, + Timeouts::set_all(Duration::from_secs( + config.chain.checkpoint_sync_url_timeout, + )), + ); let slots_per_epoch = TEthSpec::slots_per_epoch(); let deposit_snapshot = if config.sync_eth1_chain { diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 81a7c6bbeb3..16a6794f432 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -714,6 +714,14 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .takes_value(true) .conflicts_with("checkpoint-state") ) + .arg( + Arg::with_name("checkpoint-sync-url-timeout") + .long("checkpoint-sync-url-timeout") + .help("Set the timeout for checkpoint sync calls to remote beacon node HTTP endpoint.") + .value_name("SECONDS") + .takes_value(true) + .default_value("60") + ) .arg( Arg::with_name("reconstruct-historic-states") .long("reconstruct-historic-states") diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 3b94c312901..6af753afea9 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -441,6 +441,8 @@ pub fn get_config( .extend_from_slice(boot_nodes) } } + client_config.chain.checkpoint_sync_url_timeout = + clap_utils::parse_required::(cli_args, "checkpoint-sync-url-timeout")?; client_config.genesis = if let Some(genesis_state_bytes) = eth2_network_config.genesis_state_bytes.clone() diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index b1498f109d6..f24ba6895ee 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -132,6 +132,25 @@ fn fork_choice_before_proposal_timeout_zero() { .with_config(|config| assert_eq!(config.chain.fork_choice_before_proposal_timeout_ms, 0)); } +#[test] +fn checkpoint_sync_url_timeout_flag() { + CommandLineTest::new() + .flag("checkpoint-sync-url-timeout", Some("300")) + .run_with_zero_port() + .with_config(|config| { + assert_eq!(config.chain.checkpoint_sync_url_timeout, 300); + }); +} + +#[test] +fn checkpoint_sync_url_timeout_default() { + CommandLineTest::new() + .run_with_zero_port() + .with_config(|config| { + assert_eq!(config.chain.checkpoint_sync_url_timeout, 60); + }); +} + #[test] fn paranoid_block_proposal_default() { CommandLineTest::new()