From 3dedab24f4ea8789f54562c0263657bb0822f10d Mon Sep 17 00:00:00 2001 From: zhiqiangxu <652732310@qq.com> Date: Thu, 1 Feb 2024 15:36:34 +0800 Subject: [PATCH 1/8] make `execution-endpoint` mandatory --- beacon_node/src/cli.rs | 1 + beacon_node/src/config.rs | 176 +++++++++++++++++++------------------- 2 files changed, 89 insertions(+), 88 deletions(-) diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 002bb344a3e..9d5a89acc85 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -634,6 +634,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { JSON-RPC connection. Uses the same endpoint to populate the \ deposit cache.") .takes_value(true) + .required(true) ) .arg( Arg::with_name("execution-jwt") diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index fff384e195f..94a81100ede 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -263,100 +263,100 @@ pub fn get_config( client_config.eth1.cache_follow_distance = Some(follow_distance); } - if let Some(endpoints) = cli_args.value_of("execution-endpoint") { - let mut el_config = execution_layer::Config::default(); - - // Always follow the deposit contract when there is an execution endpoint. - // - // This is wasteful for non-staking nodes as they have no need to process deposit contract - // logs and build an "eth1" cache. The alternative is to explicitly require the `--eth1` or - // `--staking` flags, however that poses a risk to stakers since they cannot produce blocks - // without "eth1". - // - // The waste for non-staking nodes is relatively small so we err on the side of safety for - // stakers. The merge is already complicated enough. - client_config.sync_eth1_chain = true; + let endpoints = cli_args + .value_of("execution-endpoint") + .expect("safe since execution-endpoint is now mandatory"); + let mut el_config = execution_layer::Config::default(); - // Parse a single execution endpoint, logging warnings if multiple endpoints are supplied. - let execution_endpoint = - parse_only_one_value(endpoints, 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. - - let secret_file: PathBuf; - // Parse a single JWT secret from a given file_path, logging warnings if multiple are supplied. - if let Some(secret_files) = cli_args.value_of("execution-jwt") { - secret_file = - parse_only_one_value(secret_files, PathBuf::from_str, "--execution-jwt", log)?; - - // 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.value_of("execution-jwt-secret-key") { - use std::fs::File; - use std::io::Write; - secret_file = client_config.data_dir().join(DEFAULT_JWT_FILE); - let mut jwt_secret_key_file = File::create(secret_file.clone()) - .map_err(|e| format!("Error while creating jwt_secret_key file: {:?}", e))?; - jwt_secret_key_file - .write_all(jwt_secret_key.as_bytes()) - .map_err(|e| { - format!( - "Error occurred while writing to jwt_secret_key file: {:?}", - e - ) - })?; - } else { - return Err("Error! Please set either --execution-jwt file_path or --execution-jwt-secret-key directly via cli when using --execution-endpoint".to_string()); - } + // Always follow the deposit contract when there is an execution endpoint. + // + // This is wasteful for non-staking nodes as they have no need to process deposit contract + // logs and build an "eth1" cache. The alternative is to explicitly require the `--eth1` or + // `--staking` flags, however that poses a risk to stakers since they cannot produce blocks + // without "eth1". + // + // The waste for non-staking nodes is relatively small so we err on the side of safety for + // stakers. The merge is already complicated enough. + client_config.sync_eth1_chain = true; + + // Parse a single execution endpoint, logging warnings if multiple endpoints are supplied. + let execution_endpoint = + parse_only_one_value(endpoints, 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. + + let secret_file: PathBuf; + // Parse a single JWT secret from a given file_path, logging warnings if multiple are supplied. + if let Some(secret_files) = cli_args.value_of("execution-jwt") { + secret_file = + parse_only_one_value(secret_files, PathBuf::from_str, "--execution-jwt", log)?; + + // 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.value_of("execution-jwt-secret-key") { + use std::fs::File; + use std::io::Write; + secret_file = client_config.data_dir().join(DEFAULT_JWT_FILE); + let mut jwt_secret_key_file = File::create(secret_file.clone()) + .map_err(|e| format!("Error while creating jwt_secret_key file: {:?}", e))?; + jwt_secret_key_file + .write_all(jwt_secret_key.as_bytes()) + .map_err(|e| { + format!( + "Error occurred while writing to jwt_secret_key file: {:?}", + e + ) + })?; + } else { + return Err("Error! Please set either --execution-jwt file_path or --execution-jwt-secret-key directly via cli when using --execution-endpoint".to_string()); + } - // Parse and set the payload builder, if any. - if let Some(endpoint) = cli_args.value_of("builder") { - let payload_builder = - parse_only_one_value(endpoint, SensitiveUrl::parse, "--builder", log)?; - el_config.builder_url = Some(payload_builder); + // Parse and set the payload builder, if any. + if let Some(endpoint) = cli_args.value_of("builder") { + let payload_builder = + parse_only_one_value(endpoint, SensitiveUrl::parse, "--builder", log)?; + el_config.builder_url = Some(payload_builder); - el_config.builder_user_agent = - clap_utils::parse_optional(cli_args, "builder-user-agent")?; - } + el_config.builder_user_agent = clap_utils::parse_optional(cli_args, "builder-user-agent")?; + } - if cli_args.is_present("builder-profit-threshold") { - warn!( - log, - "Ignoring --builder-profit-threshold"; - "info" => "this flag is deprecated and will be removed" - ); - } - if cli_args.is_present("always-prefer-builder-payload") { - warn!( - log, - "Ignoring --always-prefer-builder-payload"; - "info" => "this flag is deprecated and will be removed" - ); - } + if cli_args.is_present("builder-profit-threshold") { + warn!( + log, + "Ignoring --builder-profit-threshold"; + "info" => "this flag is deprecated and will be removed" + ); + } + if cli_args.is_present("always-prefer-builder-payload") { + warn!( + log, + "Ignoring --always-prefer-builder-payload"; + "info" => "this flag is deprecated and will be removed" + ); + } - // Set config values from parse values. - el_config.secret_files = vec![secret_file.clone()]; - el_config.execution_endpoints = vec![execution_endpoint.clone()]; - el_config.suggested_fee_recipient = - clap_utils::parse_optional(cli_args, "suggested-fee-recipient")?; - el_config.jwt_id = clap_utils::parse_optional(cli_args, "execution-jwt-id")?; - el_config.jwt_version = clap_utils::parse_optional(cli_args, "execution-jwt-version")?; - el_config.default_datadir = client_config.data_dir().clone(); - let execution_timeout_multiplier = - clap_utils::parse_required(cli_args, "execution-timeout-multiplier")?; - el_config.execution_timeout_multiplier = Some(execution_timeout_multiplier); - - client_config.eth1.endpoint = Eth1Endpoint::Auth { - endpoint: execution_endpoint, - jwt_path: secret_file, - jwt_id: el_config.jwt_id.clone(), - jwt_version: el_config.jwt_version.clone(), - }; + // Set config values from parse values. + el_config.secret_files = vec![secret_file.clone()]; + el_config.execution_endpoints = vec![execution_endpoint.clone()]; + el_config.suggested_fee_recipient = + clap_utils::parse_optional(cli_args, "suggested-fee-recipient")?; + el_config.jwt_id = clap_utils::parse_optional(cli_args, "execution-jwt-id")?; + el_config.jwt_version = clap_utils::parse_optional(cli_args, "execution-jwt-version")?; + el_config.default_datadir = client_config.data_dir().clone(); + let execution_timeout_multiplier = + clap_utils::parse_required(cli_args, "execution-timeout-multiplier")?; + el_config.execution_timeout_multiplier = Some(execution_timeout_multiplier); + + client_config.eth1.endpoint = Eth1Endpoint::Auth { + endpoint: execution_endpoint, + jwt_path: secret_file, + jwt_id: el_config.jwt_id.clone(), + jwt_version: el_config.jwt_version.clone(), + }; - // Store the EL config in the client config. - client_config.execution_layer = Some(el_config); - } + // Store the EL config in the client config. + client_config.execution_layer = Some(el_config); // 4844 params client_config.trusted_setup = context From baf755b35e5140391caf4c4e1818f614dc519d45 Mon Sep 17 00:00:00 2001 From: zhiqiangxu <652732310@qq.com> Date: Thu, 1 Feb 2024 20:42:05 +0800 Subject: [PATCH 2/8] use parse_required instead --- beacon_node/src/config.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 94a81100ede..63d359ce08a 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -263,9 +263,7 @@ pub fn get_config( client_config.eth1.cache_follow_distance = Some(follow_distance); } - let endpoints = cli_args - .value_of("execution-endpoint") - .expect("safe since execution-endpoint is now mandatory"); + 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. @@ -280,8 +278,12 @@ pub fn get_config( client_config.sync_eth1_chain = true; // Parse a single execution endpoint, logging warnings if multiple endpoints are supplied. - let execution_endpoint = - parse_only_one_value(endpoints, SensitiveUrl::parse, "--execution-endpoint", log)?; + let execution_endpoint = parse_only_one_value( + 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. From 486686dbc512d107def463768f73b10ef2ae07ef Mon Sep 17 00:00:00 2001 From: zhiqiangxu <652732310@qq.com> Date: Thu, 22 Feb 2024 13:22:52 +0800 Subject: [PATCH 3/8] make test pass --- lighthouse/tests/exec.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lighthouse/tests/exec.rs b/lighthouse/tests/exec.rs index 61e0677ca8c..28d1fe95b88 100644 --- a/lighthouse/tests/exec.rs +++ b/lighthouse/tests/exec.rs @@ -41,6 +41,19 @@ pub trait CommandLineTestExec { .arg(format!("--{}", "dump-chain-config")) .arg(tmp_chain_config_path.as_os_str()) .arg("--immediate-shutdown"); + if !cmd.get_args().any(|arg| arg == "--execution-endpoint") { + cmd.arg("--execution-endpoint").arg("http://localhost"); + } + if !cmd.get_args().any(|arg| { + arg == "--execution-jwt" + || arg == "--execution-jwt-secret-key" + || arg == "--jwt-file" + || arg == "--jwt-secrets" + }) { + // jwt-secret-key length should be 64 + cmd.arg("--execution-jwt-secret-key") + .arg("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); + } // Run the command. let output = output_result(cmd); From bc30999330d48e2be385a1ab3b1d6cadc343bf5c Mon Sep 17 00:00:00 2001 From: alan <652732310@qq.com> Date: Fri, 28 Jun 2024 22:42:30 +0800 Subject: [PATCH 4/8] fix test --- lighthouse/tests/exec.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lighthouse/tests/exec.rs b/lighthouse/tests/exec.rs index 2d4dc65b312..a0d052eab4c 100644 --- a/lighthouse/tests/exec.rs +++ b/lighthouse/tests/exec.rs @@ -47,7 +47,10 @@ pub trait CommandLineTestExec { .arg(tmp_config_path.as_os_str()) .arg(format!("--{}", "dump-chain-config")) .arg(tmp_chain_config_path.as_os_str()); - if !cmd.get_args().any(|arg| arg == "--execution-endpoint") { + if !cmd + .get_args() + .any(|arg| arg == "--execution-endpoint" || arg == "--execution-endpoints") + { cmd.arg("--execution-endpoint").arg("http://localhost"); } if !cmd.get_args().any(|arg| { From 93bc19bd8a16bccceac776493697d80fe49dc090 Mon Sep 17 00:00:00 2001 From: Mac L Date: Fri, 18 Oct 2024 04:27:48 +1100 Subject: [PATCH 5/8] Fix cli help text --- book/src/help_bn.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/book/src/help_bn.md b/book/src/help_bn.md index 733446e5d27..bed8eff8b79 100644 --- a/book/src/help_bn.md +++ b/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 Options: --auto-compact-db From d745d090b5a0b40b9ba86ec3d208a634054ed39c Mon Sep 17 00:00:00 2001 From: Mac L Date: Sat, 26 Oct 2024 04:20:07 +1100 Subject: [PATCH 6/8] Fix tests --- lighthouse/tests/beacon_node.rs | 39 ++++++++++++++++++++++----------- lighthouse/tests/exec.rs | 16 -------------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index f3832a1a1e5..39e3ddafaaf 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -24,7 +24,9 @@ use types::non_zero_usize::new_non_zero_usize; use types::{Address, Checkpoint, Epoch, ExecutionBlockHash, 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:8545/"; +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,17 @@ 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 } + } + + fn new_with_no_execution_endpoint() -> CommandLineTest { let base_cmd = base_cmd(); CommandLineTest { cmd: base_cmd } } @@ -104,7 +117,7 @@ fn staking_flag() { assert!(config.sync_eth1_chain); assert_eq!( config.eth1.endpoint.get_endpoint().to_string(), - DEFAULT_ETH1_ENDPOINT + DEFAULT_EXECUTION_ENDPOINT ); }); } @@ -260,7 +273,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", @@ -466,7 +479,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() @@ -487,7 +500,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() @@ -508,7 +521,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", @@ -535,7 +548,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", @@ -557,7 +570,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", @@ -598,7 +611,7 @@ fn run_payload_builder_flag_test_with_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", @@ -720,7 +733,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)) @@ -2505,13 +2518,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", @@ -2524,7 +2537,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( diff --git a/lighthouse/tests/exec.rs b/lighthouse/tests/exec.rs index a0d052eab4c..9d6453908c8 100644 --- a/lighthouse/tests/exec.rs +++ b/lighthouse/tests/exec.rs @@ -47,22 +47,6 @@ pub trait CommandLineTestExec { .arg(tmp_config_path.as_os_str()) .arg(format!("--{}", "dump-chain-config")) .arg(tmp_chain_config_path.as_os_str()); - if !cmd - .get_args() - .any(|arg| arg == "--execution-endpoint" || arg == "--execution-endpoints") - { - cmd.arg("--execution-endpoint").arg("http://localhost"); - } - if !cmd.get_args().any(|arg| { - arg == "--execution-jwt" - || arg == "--execution-jwt-secret-key" - || arg == "--jwt-file" - || arg == "--jwt-secrets" - }) { - // jwt-secret-key length should be 64 - cmd.arg("--execution-jwt-secret-key") - .arg("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); - } if immediate_shutdown { cmd.arg("--immediate-shutdown"); From dae79a59cb9912e2cf405ea828926f00810d9f84 Mon Sep 17 00:00:00 2001 From: Mac L Date: Mon, 28 Oct 2024 04:11:24 +1100 Subject: [PATCH 7/8] Add comment --- lighthouse/tests/beacon_node.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index 4d400a928b0..f77728ca6f6 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -64,6 +64,7 @@ impl CommandLineTest { 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 } From 1d7694f052619be848a7d3c18616317c9e0e92d9 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Fri, 1 Nov 2024 15:36:46 +1100 Subject: [PATCH 8/8] Clarification --- lighthouse/tests/beacon_node.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index f77728ca6f6..ffa6e300a75 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -24,7 +24,7 @@ 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_EXECUTION_ENDPOINT: &str = "http://localhost:8545/"; +const DEFAULT_EXECUTION_ENDPOINT: &str = "http://localhost:8551/"; const DEFAULT_EXECUTION_JWT_SECRET_KEY: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";