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 <micsproul@gmail.com>
sigp · Nov 11, 2022 · c591fcd · c591fcd
1 parent d99bfcf
commit c591fcd
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -441,6 +441,8 @@ pub fn get_config<E: EthSpec>(
                 .extend_from_slice(boot_nodes)
         }
     }
+    client_config.chain.checkpoint_sync_url_timeout =
+        clap_utils::parse_required::<u64>(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
@@ -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()