Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate http-spec-fork and http-allow-sync-stalled #5500

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions beacon_node/http_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ pub struct Config {
pub listen_port: u16,
pub allow_origin: Option<String>,
pub tls_config: Option<TlsConfig>,
pub allow_sync_stalled: bool,
pub spec_fork_name: Option<ForkName>,
pub data_dir: PathBuf,
pub sse_capacity_multiplier: usize,
Expand All @@ -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,
Expand Down Expand Up @@ -321,7 +319,6 @@ pub fn serve<T: BeaconChainTypes>(
shutdown: impl Future<Output = ()> + Send + Sync + 'static,
) -> Result<HttpServer, Error> {
let config = ctx.config.clone();
let allow_sync_stalled = config.allow_sync_stalled;
let log = ctx.log.clone();

// Configure CORS.
Expand Down Expand Up @@ -482,10 +479,8 @@ pub fn serve<T: BeaconChainTypes>(
| 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(()),

}
},
)
Expand Down
9 changes: 4 additions & 5 deletions beacon_node/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
8 changes: 0 additions & 8 deletions beacon_node/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,6 @@ pub fn get_config<E: EthSpec>(
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);
michaelsproul marked this conversation as resolved.
Show resolved Hide resolved
}

if cli_args.is_present("http-enable-tls") {
client_config.http_api.tls_config = Some(TlsConfig {
cert: cli_args
Expand All @@ -146,10 +142,6 @@ pub fn get_config<E: EthSpec>(
});
}

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")?;

Expand Down
4 changes: 2 additions & 2 deletions book/src/help_bn.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
eserilev marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -281,7 +281,7 @@ OPTIONS:
Set the listen TCP port for the RESTful HTTP API server.

--http-spec-fork <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 <N>
Multiplier to apply to the length of HTTP server-sent-event (SSE) channels. Increasing this value can
Expand Down
20 changes: 2 additions & 18 deletions lighthouse/tests/beacon_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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() {
Expand Down
1 change: 0 additions & 1 deletion scripts/local_testnet/beacon_node.sh
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,4 @@ exec $lighthouse_binary \
--target-peers $((BN_COUNT - 1)) \
--execution-endpoint $execution_endpoint \
--execution-jwt $execution_jwt \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to remove the backlash in the previous line?

Suggested change
--execution-jwt $execution_jwt \
--execution-jwt $execution_jwt

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, removed in a3e4f5e

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, we need to keep it because $BN_ARGS is on the next line

--http-allow-sync-stalled \
$BN_ARGS
2 changes: 0 additions & 2 deletions testing/simulator/src/sync_sim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading