From 275c6cbf5d676db5a68d6679ad62b50aed4901ea Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Thu, 28 Mar 2024 00:15:17 +0200 Subject: [PATCH 1/7] deprecate flags --- beacon_node/http_api/src/lib.rs | 9 ++------- beacon_node/src/cli.rs | 9 ++++----- beacon_node/src/config.rs | 8 -------- book/src/help_bn.md | 4 ++-- lighthouse/tests/beacon_node.rs | 20 ++------------------ scripts/local_testnet/beacon_node.sh | 1 - testing/simulator/src/sync_sim.rs | 2 -- 7 files changed, 10 insertions(+), 43 deletions(-) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 6d421e7c456..1ebba9bda80 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -144,7 +144,6 @@ pub struct Config { pub listen_port: u16, pub allow_origin: Option, pub tls_config: Option, - pub allow_sync_stalled: bool, pub spec_fork_name: Option, pub data_dir: PathBuf, pub sse_capacity_multiplier: usize, @@ -162,7 +161,6 @@ impl Default for Config { listen_port: 5052, allow_origin: None, tls_config: None, - allow_sync_stalled: false, spec_fork_name: None, data_dir: PathBuf::from(DEFAULT_ROOT_DIR), sse_capacity_multiplier: 1, @@ -321,7 +319,6 @@ pub fn serve( shutdown: impl Future + Send + Sync + 'static, ) -> Result { let config = ctx.config.clone(); - let allow_sync_stalled = config.allow_sync_stalled; let log = ctx.log.clone(); // Configure CORS. @@ -482,10 +479,8 @@ pub fn serve( | SyncState::SyncTransition | SyncState::BackFillSyncing { .. } => Ok(()), SyncState::Synced => Ok(()), - SyncState::Stalled if allow_sync_stalled => Ok(()), - SyncState::Stalled => Err(warp_utils::reject::not_synced( - "sync is stalled".to_string(), - )), + SyncState::Stalled => Ok(()), + } }, ) diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index d3f2e051dd2..bae5cbac099 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -393,9 +393,9 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .long("http-spec-fork") .requires("enable_http") .value_name("FORK") - .help("Serve the spec for a specific hard fork on /eth/v1/config/spec. It should \ - not be necessary to set this flag.") + .help("This flag is deprecated and has no effect.") .takes_value(true) + .hidden(true) ) .arg( Arg::with_name("http-enable-tls") @@ -426,9 +426,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { Arg::with_name("http-allow-sync-stalled") .long("http-allow-sync-stalled") .requires("enable_http") - .help("Forces the HTTP to indicate that the node is synced when sync is actually \ - stalled. This is useful for very small testnets. TESTING ONLY. DO NOT USE ON \ - MAINNET.") + .help("This flag is deprecated and has no effect.") + .hidden(true) ) .arg( Arg::with_name("http-sse-capacity-multiplier") diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 8f369818c09..a04a0158396 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -127,10 +127,6 @@ pub fn get_config( client_config.http_api.allow_origin = Some(allow_origin.to_string()); } - if let Some(fork_name) = clap_utils::parse_optional(cli_args, "http-spec-fork")? { - client_config.http_api.spec_fork_name = Some(fork_name); - } - if cli_args.is_present("http-enable-tls") { client_config.http_api.tls_config = Some(TlsConfig { cert: cli_args @@ -146,10 +142,6 @@ pub fn get_config( }); } - if cli_args.is_present("http-allow-sync-stalled") { - client_config.http_api.allow_sync_stalled = true; - } - client_config.http_api.sse_capacity_multiplier = parse_required(cli_args, "http-sse-capacity-multiplier")?; diff --git a/book/src/help_bn.md b/book/src/help_bn.md index c0505988ce1..97865bc73f9 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -70,7 +70,7 @@ FLAGS: enables --http and --validator-monitor-auto and enables SSE logging. -h, --help Prints help information --http Enable the RESTful HTTP API server. Disabled by default. - --http-allow-sync-stalled Forces the HTTP to indicate that the node is synced when sync is actually + --http-allow-sync-stalled This flag is deprecated and has no effect. stalled. This is useful for very small testnets. TESTING ONLY. DO NOT USE ON MAINNET. --http-enable-tls Serves the RESTful HTTP API server over TLS. This feature is currently @@ -281,7 +281,7 @@ OPTIONS: Set the listen TCP port for the RESTful HTTP API server. --http-spec-fork - Serve the spec for a specific hard fork on /eth/v1/config/spec. It should not be necessary to set this flag. + This flag is deprecated and has no effect. --http-sse-capacity-multiplier Multiplier to apply to the length of HTTP server-sent-event (SSE) channels. Increasing this value can diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index b6cd84a69fc..ffb2d2aa2fd 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -20,7 +20,7 @@ use std::time::Duration; use tempfile::TempDir; use types::non_zero_usize::new_non_zero_usize; use types::{ - Address, Checkpoint, Epoch, ExecutionBlockHash, ForkName, Hash256, MainnetEthSpec, + Address, Checkpoint, Epoch, ExecutionBlockHash, Hash256, MainnetEthSpec, ProgressiveBalancesMode, }; use unused_port::{unused_tcp4_port, unused_tcp6_port, unused_udp4_port, unused_udp6_port}; @@ -1586,14 +1586,7 @@ fn http_allow_origin_all_flag() { .run_with_zero_port() .with_config(|config| assert_eq!(config.http_api.allow_origin, Some("*".to_string()))); } -#[test] -fn http_allow_sync_stalled_flag() { - CommandLineTest::new() - .flag("http", None) - .flag("http-allow-sync-stalled", None) - .run_with_zero_port() - .with_config(|config| assert_eq!(config.http_api.allow_sync_stalled, true)); -} + #[test] fn http_enable_beacon_processor() { CommandLineTest::new() @@ -1640,15 +1633,6 @@ fn http_spec_fork_default() { .with_config(|config| assert_eq!(config.http_api.spec_fork_name, None)); } -#[test] -fn http_spec_fork_override() { - CommandLineTest::new() - .flag("http", None) - .flag("http-spec-fork", Some("altair")) - .run_with_zero_port() - .with_config(|config| assert_eq!(config.http_api.spec_fork_name, Some(ForkName::Altair))); -} - // Tests for Metrics flags. #[test] fn metrics_flag() { diff --git a/scripts/local_testnet/beacon_node.sh b/scripts/local_testnet/beacon_node.sh index 2660dfa3c00..940fe2b8581 100755 --- a/scripts/local_testnet/beacon_node.sh +++ b/scripts/local_testnet/beacon_node.sh @@ -67,5 +67,4 @@ exec $lighthouse_binary \ --target-peers $((BN_COUNT - 1)) \ --execution-endpoint $execution_endpoint \ --execution-jwt $execution_jwt \ - --http-allow-sync-stalled \ $BN_ARGS diff --git a/testing/simulator/src/sync_sim.rs b/testing/simulator/src/sync_sim.rs index 78f7e1ee9fb..ba4ea4530af 100644 --- a/testing/simulator/src/sync_sim.rs +++ b/testing/simulator/src/sync_sim.rs @@ -94,8 +94,6 @@ fn syncing_sim( beacon_config.dummy_eth1_backend = true; beacon_config.sync_eth1_chain = true; - beacon_config.http_api.allow_sync_stalled = true; - beacon_config.network.enr_address = (Some(Ipv4Addr::LOCALHOST), None); // Generate the directories and keystores required for the validator clients. From 84120c41b2f344368030c4c07650a2d5ec1e239f Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Thu, 28 Mar 2024 01:22:30 +0200 Subject: [PATCH 2/7] fmt --- beacon_node/http_api/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 1ebba9bda80..3e86a8e7907 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -480,7 +480,6 @@ pub fn serve( | SyncState::BackFillSyncing { .. } => Ok(()), SyncState::Synced => Ok(()), SyncState::Stalled => Ok(()), - } }, ) From a3e4f5e57661640efc778b37dca509c8484700b5 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Fri, 29 Mar 2024 01:20:50 +0200 Subject: [PATCH 3/7] remove backslash --- scripts/local_testnet/beacon_node.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/local_testnet/beacon_node.sh b/scripts/local_testnet/beacon_node.sh index 940fe2b8581..b20dbd81281 100755 --- a/scripts/local_testnet/beacon_node.sh +++ b/scripts/local_testnet/beacon_node.sh @@ -66,5 +66,5 @@ exec $lighthouse_binary \ --disable-packet-filter \ --target-peers $((BN_COUNT - 1)) \ --execution-endpoint $execution_endpoint \ - --execution-jwt $execution_jwt \ + --execution-jwt $execution_jwt $BN_ARGS From da14923c32473518fd9b82d6fd272d7be4bf47e4 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Thu, 4 Apr 2024 01:00:45 +0300 Subject: [PATCH 4/7] remove hidden flags from the book --- book/src/help_bn.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/book/src/help_bn.md b/book/src/help_bn.md index 97865bc73f9..0644925fc87 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -70,9 +70,6 @@ FLAGS: enables --http and --validator-monitor-auto and enables SSE logging. -h, --help Prints help information --http Enable the RESTful HTTP API server. Disabled by default. - --http-allow-sync-stalled This flag is deprecated and has no effect. - stalled. This is useful for very small testnets. TESTING ONLY. DO NOT USE - ON MAINNET. --http-enable-tls Serves the RESTful HTTP API server over TLS. This feature is currently experimental. --import-all-attestations Import and aggregate all attestations, regardless of validator @@ -280,9 +277,6 @@ OPTIONS: --http-port Set the listen TCP port for the RESTful HTTP API server. - --http-spec-fork - This flag is deprecated and has no effect. - --http-sse-capacity-multiplier Multiplier to apply to the length of HTTP server-sent-event (SSE) channels. Increasing this value can prevent messages from being dropped. From 60bd5745a9a024813c0eed6ebd827e1c3031ce2d Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 10 Apr 2024 11:56:31 +0300 Subject: [PATCH 5/7] add warn, re-add tests --- beacon_node/src/cli.rs | 2 +- beacon_node/src/config.rs | 16 ++++++++++++++++ lighthouse/tests/beacon_node.rs | 16 ++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index bae5cbac099..7eb6349fbd4 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -383,7 +383,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .requires("enable_http") .value_name("ORIGIN") .help("Set the value of the Access-Control-Allow-Origin response HTTP header. \ - Use * to allow any origin (not recommended in production). \ + Use * to allow any origin (not recommended in production). If no value is supplied, the CORS allowed origin is set to the listen \ address of this server (e.g., http://localhost:5052).") .takes_value(true), diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 1d2b9689a68..27b34088a71 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -127,6 +127,14 @@ pub fn get_config( client_config.http_api.allow_origin = Some(allow_origin.to_string()); } + if cli_args.is_present("http-spec-fork") { + warn!( + log, + "Ignoring --http-spec-fork"; + "info" => "this flag is deprecated and will be removed" + ); + } + if cli_args.is_present("http-enable-tls") { client_config.http_api.tls_config = Some(TlsConfig { cert: cli_args @@ -142,6 +150,14 @@ pub fn get_config( }); } + if cli_args.is_present("http-allow-sync-stalled") { + warn!( + log, + "Ignoring --http-allow-sync-stalled"; + "info" => "this flag is deprecated and will be removed" + ); + } + client_config.http_api.sse_capacity_multiplier = parse_required(cli_args, "http-sse-capacity-multiplier")?; diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index ffb2d2aa2fd..d48c39b47b9 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -1587,6 +1587,14 @@ fn http_allow_origin_all_flag() { .with_config(|config| assert_eq!(config.http_api.allow_origin, Some("*".to_string()))); } +#[test] +fn http_allow_sync_stalled_flag() { + CommandLineTest::new() + .flag("http", None) + .flag("http-allow-sync-stalled", None) + .run_with_zero_port(); +} + #[test] fn http_enable_beacon_processor() { CommandLineTest::new() @@ -1633,6 +1641,14 @@ fn http_spec_fork_default() { .with_config(|config| assert_eq!(config.http_api.spec_fork_name, None)); } +#[test] +fn http_spec_fork_override() { + CommandLineTest::new() + .flag("http", None) + .flag("http-spec-fork", Some("altair")) + .run_with_zero_port(); +} + // Tests for Metrics flags. #[test] fn metrics_flag() { From 0193496885feb496acdda420c89e0864c3bc407c Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 11 Apr 2024 09:24:55 +1000 Subject: [PATCH 6/7] Apply suggestions from code review --- beacon_node/src/cli.rs | 2 +- scripts/local_testnet/beacon_node.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 7eb6349fbd4..bae5cbac099 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -383,7 +383,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .requires("enable_http") .value_name("ORIGIN") .help("Set the value of the Access-Control-Allow-Origin response HTTP header. \ - Use * to allow any origin (not recommended in production). + Use * to allow any origin (not recommended in production). \ If no value is supplied, the CORS allowed origin is set to the listen \ address of this server (e.g., http://localhost:5052).") .takes_value(true), diff --git a/scripts/local_testnet/beacon_node.sh b/scripts/local_testnet/beacon_node.sh index b20dbd81281..940fe2b8581 100755 --- a/scripts/local_testnet/beacon_node.sh +++ b/scripts/local_testnet/beacon_node.sh @@ -66,5 +66,5 @@ exec $lighthouse_binary \ --disable-packet-filter \ --target-peers $((BN_COUNT - 1)) \ --execution-endpoint $execution_endpoint \ - --execution-jwt $execution_jwt + --execution-jwt $execution_jwt \ $BN_ARGS From 908d5c6390c18f5b4ddd2ca1b07e042af209411e Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 11 Apr 2024 10:10:20 +1000 Subject: [PATCH 7/7] Fix imports --- 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 54738d6fdc4..4a50126945b 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -19,7 +19,7 @@ use std::string::ToString; use std::time::Duration; use tempfile::TempDir; use types::non_zero_usize::new_non_zero_usize; -use types::{Address, Checkpoint, Epoch, ExecutionBlockHash, ForkName, Hash256, MainnetEthSpec}; +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/";