Add builder header timeout flag (#5857)

* fix bitvector test random impl

* add a new flag that allows for configuring the timeout for get builder header api calls

* make cli

* add upper limit check, changes based on feedback

* update cli

* capitalization

* cli fix

beacon_node/beacon_chain/src/test_utils.rs

@@ -684,6 +684,7 @@ where
             .set_builder_url(
                 SensitiveUrl::parse(format!("http://127.0.0.1:{port}").as_str()).unwrap(),
                 None,
+                None,
             )
             .unwrap();
 

beacon_node/builder_client/src/lib.rs

@@ -29,10 +29,13 @@ pub struct Timeouts {
     get_builder_status: Duration,
 }
 
-impl Default for Timeouts {
-    fn default() -> Self {
+impl Timeouts {
+    fn new(get_header_timeout: Option<Duration>) -> Self {
+        let get_header =
+            get_header_timeout.unwrap_or(Duration::from_millis(DEFAULT_GET_HEADER_TIMEOUT_MILLIS));
+
         Self {
-            get_header: Duration::from_millis(DEFAULT_GET_HEADER_TIMEOUT_MILLIS),
+            get_header,
             post_validators: Duration::from_millis(DEFAULT_TIMEOUT_MILLIS),
             post_blinded_blocks: Duration::from_millis(DEFAULT_TIMEOUT_MILLIS),
             get_builder_status: Duration::from_millis(DEFAULT_TIMEOUT_MILLIS),
@@ -49,13 +52,17 @@ pub struct BuilderHttpClient {
 }
 
 impl BuilderHttpClient {
-    pub fn new(server: SensitiveUrl, user_agent: Option<String>) -> Result<Self, Error> {
+    pub fn new(
+        server: SensitiveUrl,
+        user_agent: Option<String>,
+        builder_header_timeout: Option<Duration>,
+    ) -> Result<Self, Error> {
         let user_agent = user_agent.unwrap_or(DEFAULT_USER_AGENT.to_string());
         let client = reqwest::Client::builder().user_agent(&user_agent).build()?;
         Ok(Self {
             client,
             server,
-            timeouts: Timeouts::default(),
+            timeouts: Timeouts::new(builder_header_timeout),
             user_agent,
         })
     }

beacon_node/execution_layer/src/lib.rs

@@ -370,6 +370,9 @@ pub struct Config {
     pub execution_endpoint: Option<SensitiveUrl>,
     /// Endpoint urls for services providing the builder api.
     pub builder_url: Option<SensitiveUrl>,
+    /// The timeout value used when making a request to fetch a block header
+    /// from the builder api.
+    pub builder_header_timeout: Option<Duration>,
     /// User agent to send with requests to the builder API.
     pub builder_user_agent: Option<String>,
     /// JWT secret for the above endpoint running the engine api.
@@ -400,6 +403,7 @@ impl<E: EthSpec> ExecutionLayer<E> {
             execution_endpoint: url,
             builder_url,
             builder_user_agent,
+            builder_header_timeout,
             secret_file,
             suggested_fee_recipient,
             jwt_id,
@@ -469,7 +473,7 @@ impl<E: EthSpec> ExecutionLayer<E> {
         };
 
         if let Some(builder_url) = builder_url {
-            el.set_builder_url(builder_url, builder_user_agent)?;
+            el.set_builder_url(builder_url, builder_user_agent, builder_header_timeout)?;
         }
 
         Ok(el)
@@ -491,9 +495,14 @@ impl<E: EthSpec> ExecutionLayer<E> {
         &self,
         builder_url: SensitiveUrl,
         builder_user_agent: Option<String>,
+        builder_header_timeout: Option<Duration>,
     ) -> Result<(), Error> {
-        let builder_client = BuilderHttpClient::new(builder_url.clone(), builder_user_agent)
-            .map_err(Error::Builder)?;
+        let builder_client = BuilderHttpClient::new(
+            builder_url.clone(),
+            builder_user_agent,
+            builder_header_timeout,
+        )
+        .map_err(Error::Builder)?;
         info!(
             self.log(),
             "Using external block builder";

beacon_node/src/cli.rs

@@ -1,3 +1,5 @@
+use std::time::Duration;
+
 use clap::{builder::ArgPredicate, crate_version, Arg, ArgAction, ArgGroup, Command};
 use clap_utils::{get_color_style, FLAG_HEADER};
 use strum::VariantNames;
@@ -855,6 +857,32 @@ pub fn cli_app() -> Command {
                 .action(ArgAction::Set)
                 .display_order(0)
         )
+        .arg(
+            Arg::new("builder-header-timeout")
+                .long("builder-header-timeout")
+                .value_name("MILLISECONDS")
+                .help("Defines a timeout value (in milliseconds) to use when \
+                    fetching a block header from the builder API.")
+                .default_value("1000")
+                .value_parser(|timeout: &str| {
+                    match timeout
+                        .parse::<u64>()
+                        .ok()
+                        .map(Duration::from_millis)
+                    {
+                        Some(val) =>  {
+                            if val > Duration::from_secs(3) {
+                                return Err("builder-header-timeout cannot exceed 3000ms")
+                            }
+                            Ok(timeout.to_string())
+                        },
+                        None => Err("builder-header-timeout must be a number"),
+                    }
+                })
+                .requires("builder")
+                .action(ArgAction::Set)
+                .display_order(0)
+        )
         /* Deneb settings */
         .arg(
             Arg::new("trusted-setup-file-override")

beacon_node/src/config.rs

@@ -329,6 +329,10 @@ pub fn get_config<E: EthSpec>(
 
             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);
         }
 
         if parse_flag(cli_args, "builder-profit-threshold") {

book/src/help_bn.md

@@ -43,6 +43,9 @@ Options:
           slots on the canonical chain in the past `SLOTS_PER_EPOCH`, it will
           NOT query any connected builders, and will use the local execution
           engine for payload construction. [default: 8]
+      --builder-header-timeout <MILLISECONDS>
+          Defines a timeout value (in milliseconds) to use when fetching a block
+          header from the builder API. [default: 1000]
       --builder-profit-threshold <WEI_VALUE>
           This flag is deprecated and has no effect.
       --builder-user-agent <STRING>

lighthouse/tests/beacon_node.rs

@@ -636,6 +636,26 @@ fn builder_fallback_flags() {
     );
 }
 
+#[test]
+fn builder_get_header_timeout() {
+    run_payload_builder_flag_test_with_config(
+        "builder",
+        "http://meow.cats",
+        Some("builder-header-timeout"),
+        Some("1500"),
+        |config| {
+            assert_eq!(
+                config
+                    .execution_layer
+                    .as_ref()
+                    .unwrap()
+                    .builder_header_timeout,
+                Some(Duration::from_millis(1500))
+            );
+        },
+    );
+}
+
 #[test]
 fn builder_user_agent() {
     run_payload_builder_flag_test_with_config(