make execution-endpoint required (#5165)

* make `execution-endpoint` mandatory

* use parse_required instead

* make test pass

* Merge branch 'unstable' into make_ee_required

* fix test

* Merge branch 'unstable' into make_ee_required

* Fix cli help text

* Fix tests

* Merge branch 'unstable' into make_ee_required

* Add comment

* Clarification

* Merge remote-tracking branch 'origin/unstable' into make_ee_required

beacon_node/src/cli.rs

@@ -793,6 +793,7 @@ pub fn cli_app() -> Command {
                 .help("Server endpoint for an execution layer JWT-authenticated HTTP \
                        JSON-RPC connection. Uses the same endpoint to populate the \
                        deposit cache.")
+                .required(true)
                 .action(ArgAction::Set)
                 .display_order(0)
         )

beacon_node/src/config.rs

@@ -284,7 +284,8 @@ pub fn get_config<E: EthSpec>(
         client_config.eth1.cache_follow_distance = Some(follow_distance);
     }
 
-    if let Some(endpoints) = cli_args.get_one::<String>("execution-endpoint") {
+    // `--execution-endpoint` is required now.
+    let endpoints: String = clap_utils::parse_required(cli_args, "execution-endpoint")?;
     let mut el_config = execution_layer::Config::default();
 
     // Always follow the deposit contract when there is an execution endpoint.
@@ -299,8 +300,12 @@ pub fn get_config<E: EthSpec>(
     client_config.sync_eth1_chain = true;
 
     // Parse a single execution endpoint, logging warnings if multiple endpoints are supplied.
-        let execution_endpoint =
+    let execution_endpoint = parse_only_one_value(
-            parse_only_one_value(endpoints, SensitiveUrl::parse, "--execution-endpoint", log)?;
+        endpoints.as_str(),
+        SensitiveUrl::parse,
+        "--execution-endpoint",
+        log,
+    )?;
 
     // JWTs are required if `--execution-endpoint` is supplied. They can be either passed via
     // file_path or directly as string.
@@ -313,8 +318,7 @@ pub fn get_config<E: EthSpec>(
 
     // Check if the JWT secret key is passed directly via cli flag and persist it to the default
     // file location.
-        } else if let Some(jwt_secret_key) = cli_args.get_one::<String>("execution-jwt-secret-key")
+    } else if let Some(jwt_secret_key) = cli_args.get_one::<String>("execution-jwt-secret-key") {
-        {
         use std::fs::File;
         use std::io::Write;
         secret_file = client_config.data_dir().join(DEFAULT_JWT_FILE);
@@ -338,8 +342,7 @@ pub fn get_config<E: EthSpec>(
             parse_only_one_value(endpoint, SensitiveUrl::parse, "--builder", log)?;
         el_config.builder_url = Some(payload_builder);
 
-            el_config.builder_user_agent =
+        el_config.builder_user_agent = clap_utils::parse_optional(cli_args, "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")?
@@ -369,7 +372,6 @@ pub fn get_config<E: EthSpec>(
 
     // Store the EL config in the client config.
     client_config.execution_layer = Some(el_config);
-    }
 
     // 4844 params
     if let Some(trusted_setup) = context

book/src/help_bn.md

@@ -5,7 +5,7 @@ The primary component which connects to the Ethereum 2.0 P2P network and
 downloads, verifies and stores blocks. Provides a HTTP API for querying the
 beacon chain and publishing messages to the network.
 
-Usage: lighthouse beacon_node [OPTIONS]
+Usage: lighthouse beacon_node [OPTIONS] --execution-endpoint <EXECUTION-ENDPOINT>
 
 Options:
       --auto-compact-db <auto-compact-db>

lighthouse/tests/beacon_node.rs

@@ -24,7 +24,9 @@ use types::non_zero_usize::new_non_zero_usize;
 use types::{Address, Checkpoint, Epoch, Hash256, MainnetEthSpec};
 use unused_port::{unused_tcp4_port, unused_tcp6_port, unused_udp4_port, unused_udp6_port};
 
-const DEFAULT_ETH1_ENDPOINT: &str = "http://localhost:8545/";
+const DEFAULT_EXECUTION_ENDPOINT: &str = "http://localhost:8551/";
+const DEFAULT_EXECUTION_JWT_SECRET_KEY: &str =
+    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
 
 // These dummy ports should ONLY be used for `enr-xxx-port` flags that do not bind.
 const DUMMY_ENR_TCP_PORT: u16 = 7777;
@@ -52,6 +54,18 @@ struct CommandLineTest {
 }
 impl CommandLineTest {
     fn new() -> CommandLineTest {
+        let mut base_cmd = base_cmd();
+
+        base_cmd
+            .arg("--execution-endpoint")
+            .arg(DEFAULT_EXECUTION_ENDPOINT)
+            .arg("--execution-jwt-secret-key")
+            .arg(DEFAULT_EXECUTION_JWT_SECRET_KEY);
+        CommandLineTest { cmd: base_cmd }
+    }
+
+    // Required for testing different JWT authentication methods.
+    fn new_with_no_execution_endpoint() -> CommandLineTest {
         let base_cmd = base_cmd();
         CommandLineTest { cmd: base_cmd }
     }
@@ -104,7 +118,7 @@ fn staking_flag() {
             assert!(config.sync_eth1_chain);
             assert_eq!(
                 config.eth1.endpoint.get_endpoint().to_string(),
-                DEFAULT_ETH1_ENDPOINT
+                DEFAULT_EXECUTION_ENDPOINT
             );
         });
 }
@@ -253,7 +267,7 @@ fn always_prepare_payload_default() {
 #[test]
 fn always_prepare_payload_override() {
     let dir = TempDir::new().expect("Unable to create temporary directory");
-    CommandLineTest::new()
+    CommandLineTest::new_with_no_execution_endpoint()
         .flag("always-prepare-payload", None)
         .flag(
             "suggested-fee-recipient",
@@ -459,7 +473,7 @@ fn run_bellatrix_execution_endpoints_flag_test(flag: &str) {
 
     // this is way better but intersperse is still a nightly feature :/
     // let endpoint_arg: String = urls.into_iter().intersperse(",").collect();
-    CommandLineTest::new()
+    CommandLineTest::new_with_no_execution_endpoint()
         .flag(flag, Some(&endpoint_arg))
         .flag("execution-jwt", Some(&jwts_arg))
         .run_with_zero_port()
@@ -480,7 +494,7 @@ fn run_bellatrix_execution_endpoints_flag_test(flag: &str) {
 #[test]
 fn run_execution_jwt_secret_key_is_persisted() {
     let jwt_secret_key = "0x3cbc11b0d8fa16f3344eacfd6ff6430b9d30734450e8adcf5400f88d327dcb33";
-    CommandLineTest::new()
+    CommandLineTest::new_with_no_execution_endpoint()
         .flag("execution-endpoint", Some("http://localhost:8551/"))
         .flag("execution-jwt-secret-key", Some(jwt_secret_key))
         .run_with_zero_port()
@@ -501,7 +515,7 @@ fn run_execution_jwt_secret_key_is_persisted() {
 #[test]
 fn execution_timeout_multiplier_flag() {
     let dir = TempDir::new().expect("Unable to create temporary directory");
-    CommandLineTest::new()
+    CommandLineTest::new_with_no_execution_endpoint()
         .flag("execution-endpoint", Some("http://meow.cats"))
         .flag(
             "execution-jwt",
@@ -528,7 +542,7 @@ fn bellatrix_jwt_secrets_flag() {
     let mut file = File::create(dir.path().join("jwtsecrets")).expect("Unable to create file");
     file.write_all(b"0x3cbc11b0d8fa16f3344eacfd6ff6430b9d30734450e8adcf5400f88d327dcb33")
         .expect("Unable to write to file");
-    CommandLineTest::new()
+    CommandLineTest::new_with_no_execution_endpoint()
         .flag("execution-endpoints", Some("http://localhost:8551/"))
         .flag(
             "jwt-secrets",
@@ -550,7 +564,7 @@ fn bellatrix_jwt_secrets_flag() {
 #[test]
 fn bellatrix_fee_recipient_flag() {
     let dir = TempDir::new().expect("Unable to create temporary directory");
-    CommandLineTest::new()
+    CommandLineTest::new_with_no_execution_endpoint()
         .flag("execution-endpoint", Some("http://meow.cats"))
         .flag(
             "execution-jwt",
@@ -591,7 +605,7 @@ fn run_payload_builder_flag_test_with_config<F: Fn(&Config)>(
     f: F,
 ) {
     let dir = TempDir::new().expect("Unable to create temporary directory");
-    let mut test = CommandLineTest::new();
+    let mut test = CommandLineTest::new_with_no_execution_endpoint();
     test.flag("execution-endpoint", Some("http://meow.cats"))
         .flag(
             "execution-jwt",
@@ -713,7 +727,7 @@ fn run_jwt_optional_flags_test(jwt_flag: &str, jwt_id_flag: &str, jwt_version_fl
     let jwt_file = "jwt-file";
     let id = "bn-1";
     let version = "Lighthouse-v2.1.3";
-    CommandLineTest::new()
+    CommandLineTest::new_with_no_execution_endpoint()
         .flag("execution-endpoint", Some(execution_endpoint))
         .flag(jwt_flag, dir.path().join(jwt_file).as_os_str().to_str())
         .flag(jwt_id_flag, Some(id))
@@ -2430,13 +2444,13 @@ fn logfile_format_flag() {
 fn sync_eth1_chain_default() {
     CommandLineTest::new()
         .run_with_zero_port()
-        .with_config(|config| assert_eq!(config.sync_eth1_chain, false));
+        .with_config(|config| assert_eq!(config.sync_eth1_chain, true));
 }
 
 #[test]
 fn sync_eth1_chain_execution_endpoints_flag() {
     let dir = TempDir::new().expect("Unable to create temporary directory");
-    CommandLineTest::new()
+    CommandLineTest::new_with_no_execution_endpoint()
         .flag("execution-endpoints", Some("http://localhost:8551/"))
         .flag(
             "execution-jwt",
@@ -2449,7 +2463,7 @@ fn sync_eth1_chain_execution_endpoints_flag() {
 #[test]
 fn sync_eth1_chain_disable_deposit_contract_sync_flag() {
     let dir = TempDir::new().expect("Unable to create temporary directory");
-    CommandLineTest::new()
+    CommandLineTest::new_with_no_execution_endpoint()
         .flag("disable-deposit-contract-sync", None)
         .flag("execution-endpoints", Some("http://localhost:8551/"))
         .flag(