Delete deprecated cli flags (#4906)

* Delete BN spec flag and VC beacon-node flag

* Remove warn

* slog

* add warn

* delete eth1-endpoint

* delete server from vc cli.rs

* delete server flag in config.rs

* delete delete-lockfiles in vc

* delete allow-unsynced flag in VC

* delete strict-fee-recipient in VC and warn log

* delete merge flag in bn (hidden)

* delete count-unrealized and count-unrealized-full in bn (hidden)

* delete http-disable-legacy-spec in bn (hidden)

* delete eth1-endpoint in lcli

* delete warn message lcli

* delete eth1-endpoints

* delete minify in slashing protection

* delete minify related

* Remove mut

* add back warn! log

* Indentation

* Delete count-unrealized

* Delete eth1-endpoints

* Delete eth1-endpoint test

* delete eth1-endpints test

* delete allow-unsynced test

* Add back lcli eth1-endpoint

---------

Co-authored-by: Michael Sproul <michael@sigmaprime.io>
This commit is contained in:
chonghe 2023-11-09 12:05:55 +08:00 committed by GitHub
parent 7818100777
commit 7fd9389a8c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 5 additions and 386 deletions

View File

@ -16,7 +16,6 @@ pub const EXPORT_CMD: &str = "export";
pub const IMPORT_FILE_ARG: &str = "IMPORT-FILE";
pub const EXPORT_FILE_ARG: &str = "EXPORT-FILE";
pub const MINIFY_FLAG: &str = "minify";
pub const PUBKEYS_FLAG: &str = "pubkeys";
pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
@ -31,16 +30,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.value_name("FILE")
.help("The slashing protection interchange file to import (.json)"),
)
.arg(
Arg::with_name(MINIFY_FLAG)
.long(MINIFY_FLAG)
.takes_value(true)
.possible_values(&["false", "true"])
.help(
"Deprecated: Lighthouse no longer requires minification on import \
because it always minifies",
),
),
)
.subcommand(
App::new(EXPORT_CMD)
@ -61,17 +50,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
comma-separated. All known keys will be exported if omitted",
),
)
.arg(
Arg::with_name(MINIFY_FLAG)
.long(MINIFY_FLAG)
.takes_value(true)
.default_value("false")
.possible_values(&["false", "true"])
.help(
"Minify the output file. This will make it smaller and faster to \
import, but not faster to generate.",
),
),
)
}
@ -92,7 +70,6 @@ pub fn cli_run<T: EthSpec>(
match matches.subcommand() {
(IMPORT_CMD, Some(matches)) => {
let import_filename: PathBuf = clap_utils::parse_required(matches, IMPORT_FILE_ARG)?;
let minify: Option<bool> = clap_utils::parse_optional(matches, MINIFY_FLAG)?;
let import_file = File::open(&import_filename).map_err(|e| {
format!(
"Unable to open import file at {}: {:?}",
@ -102,23 +79,10 @@ pub fn cli_run<T: EthSpec>(
})?;
eprint!("Loading JSON file into memory & deserializing");
let mut interchange = Interchange::from_json_reader(&import_file)
let interchange = Interchange::from_json_reader(&import_file)
.map_err(|e| format!("Error parsing file for import: {:?}", e))?;
eprintln!(" [done].");
if let Some(minify) = minify {
eprintln!(
"WARNING: --minify flag is deprecated and will be removed in a future release"
);
if minify {
eprint!("Minifying input file for faster loading");
interchange = interchange
.minify()
.map_err(|e| format!("Minification failed: {:?}", e))?;
eprintln!(" [done].");
}
}
let slashing_protection_database =
SlashingDatabase::open_or_create(&slashing_protection_db_path).map_err(|e| {
format!(
@ -206,7 +170,6 @@ pub fn cli_run<T: EthSpec>(
}
(EXPORT_CMD, Some(matches)) => {
let export_filename: PathBuf = clap_utils::parse_required(matches, EXPORT_FILE_ARG)?;
let minify: bool = clap_utils::parse_required(matches, MINIFY_FLAG)?;
let selected_pubkeys = if let Some(pubkeys) =
clap_utils::parse_optional::<String>(matches, PUBKEYS_FLAG)?
@ -237,17 +200,10 @@ pub fn cli_run<T: EthSpec>(
)
})?;
let mut interchange = slashing_protection_database
let interchange = slashing_protection_database
.export_interchange_info(genesis_validators_root, selected_pubkeys.as_deref())
.map_err(|e| format!("Error during export: {:?}", e))?;
if minify {
eprintln!("Minifying output file");
interchange = interchange
.minify()
.map_err(|e| format!("Unable to minify output: {:?}", e))?;
}
let output_file = File::create(export_filename)
.map_err(|e| format!("Error creating output file: {:?}", e))?;

View File

@ -388,12 +388,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
address of this server (e.g., http://localhost:5052).")
.takes_value(true),
)
.arg(
Arg::with_name("http-disable-legacy-spec")
.long("http-disable-legacy-spec")
.requires("enable_http")
.hidden(true)
)
.arg(
Arg::with_name("http-spec-fork")
.long("http-spec-fork")
@ -569,24 +563,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.help("If present, uses an eth1 backend that generates static dummy data.\
Identical to the method used at the 2019 Canada interop.")
)
.arg(
Arg::with_name("eth1-endpoint")
.long("eth1-endpoint")
.value_name("HTTP-ENDPOINT")
.help("Deprecated. Use --eth1-endpoints.")
.takes_value(true)
)
.arg(
Arg::with_name("eth1-endpoints")
.long("eth1-endpoints")
.value_name("HTTP-ENDPOINTS")
.conflicts_with("eth1-endpoint")
.help("One http endpoint for a web3 connection to an execution node. \
Note: This flag is now only useful for testing, use `--execution-endpoint` \
flag to connect to an execution node on mainnet and testnets.
Defaults to http://127.0.0.1:8545.")
.takes_value(true)
)
.arg(
Arg::with_name("eth1-purge-cache")
.long("eth1-purge-cache")
@ -649,14 +625,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
/*
* Execution Layer Integration
*/
.arg(
Arg::with_name("merge")
.long("merge")
.help("Deprecated. The feature activates automatically when --execution-endpoint \
is supplied.")
.takes_value(false)
.hidden(true)
)
.arg(
Arg::with_name("execution-endpoint")
.long("execution-endpoint")
@ -1200,22 +1168,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.requires("builder")
.takes_value(true)
)
.arg(
Arg::with_name("count-unrealized")
.long("count-unrealized")
.hidden(true)
.help("This flag is deprecated and has no effect.")
.takes_value(true)
.default_value("true")
)
.arg(
Arg::with_name("count-unrealized-full")
.long("count-unrealized-full")
.hidden(true)
.help("This flag is deprecated and has no effect.")
.takes_value(true)
.default_value("false")
)
.arg(
Arg::with_name("reset-payload-statuses")
.long("reset-payload-statuses")

View File

@ -120,13 +120,6 @@ pub fn get_config<E: EthSpec>(
client_config.http_api.allow_origin = Some(allow_origin.to_string());
}
if cli_args.is_present("http-disable-legacy-spec") {
warn!(
log,
"The flag --http-disable-legacy-spec is deprecated and will be removed"
);
}
if let Some(fork_name) = clap_utils::parse_optional(cli_args, "http-spec-fork")? {
client_config.http_api.spec_fork_name = Some(fork_name);
}
@ -240,25 +233,6 @@ pub fn get_config<E: EthSpec>(
client_config.sync_eth1_chain = true;
}
// Defines the URL to reach the eth1 node.
if let Some(endpoint) = cli_args.value_of("eth1-endpoint") {
warn!(
log,
"The --eth1-endpoint flag is deprecated";
"msg" => "please use --eth1-endpoints instead"
);
client_config.sync_eth1_chain = true;
let endpoint = SensitiveUrl::parse(endpoint)
.map_err(|e| format!("eth1-endpoint was an invalid URL: {:?}", e))?;
client_config.eth1.endpoint = Eth1Endpoint::NoAuth(endpoint);
} else if let Some(endpoint) = cli_args.value_of("eth1-endpoints") {
client_config.sync_eth1_chain = true;
let endpoint = SensitiveUrl::parse(endpoint)
.map_err(|e| format!("eth1-endpoints contains an invalid URL {:?}", e))?;
client_config.eth1.endpoint = Eth1Endpoint::NoAuth(endpoint);
}
if let Some(val) = cli_args.value_of("eth1-blocks-per-log-query") {
client_config.eth1.blocks_per_log_query = val
.parse()
@ -275,20 +249,6 @@ pub fn get_config<E: EthSpec>(
client_config.eth1.cache_follow_distance = Some(follow_distance);
}
if cli_args.is_present("merge") {
if cli_args.is_present("execution-endpoint") {
warn!(
log,
"The --merge flag is deprecated";
"info" => "the --execution-endpoint flag automatically enables this feature"
)
} else {
return Err("The --merge flag is deprecated. \
Supply a value to --execution-endpoint instead."
.into());
}
}
if let Some(endpoints) = cli_args.value_of("execution-endpoint") {
let mut el_config = execution_layer::Config::default();
@ -364,16 +324,6 @@ pub fn get_config<E: EthSpec>(
clap_utils::parse_required(cli_args, "execution-timeout-multiplier")?;
el_config.execution_timeout_multiplier = Some(execution_timeout_multiplier);
// If `--execution-endpoint` is provided, we should ignore any `--eth1-endpoints` values and
// use `--execution-endpoint` instead. Also, log a deprecation warning.
if cli_args.is_present("eth1-endpoints") || cli_args.is_present("eth1-endpoint") {
warn!(
log,
"Ignoring --eth1-endpoints flag";
"info" => "the value for --execution-endpoint will be used instead. \
--eth1-endpoints has been deprecated for post-merge configurations"
);
}
client_config.eth1.endpoint = Eth1Endpoint::Auth {
endpoint: execution_endpoint,
jwt_path: secret_file,
@ -816,22 +766,6 @@ pub fn get_config<E: EthSpec>(
client_config.chain.fork_choice_before_proposal_timeout_ms = timeout;
}
if !clap_utils::parse_required::<bool>(cli_args, "count-unrealized")? {
warn!(
log,
"The flag --count-unrealized is deprecated and will be removed";
"info" => "any use of the flag will have no effect"
);
}
if clap_utils::parse_required::<bool>(cli_args, "count-unrealized-full")? {
warn!(
log,
"The flag --count-unrealized-full is deprecated and will be removed";
"info" => "setting it to `true` has no effect"
);
}
client_config.chain.always_reset_payload_statuses =
cli_args.is_present("reset-payload-statuses");

View File

@ -11,7 +11,7 @@ use ethereum_hashing::have_sha_extensions;
use futures::TryFutureExt;
use lighthouse_version::VERSION;
use malloc_utils::configure_memory_allocator;
use slog::{crit, info, warn};
use slog::{crit, info};
use std::path::PathBuf;
use std::process::exit;
use task_executor::ShutdownReason;
@ -81,16 +81,6 @@ fn main() {
cfg!(feature = "gnosis"),
).as_str()
)
.arg(
Arg::with_name("spec")
.short("s")
.long("spec")
.value_name("DEPRECATED")
.help("This flag is deprecated, it will be disallowed in a future release. This \
value is now derived from the --network or --testnet-dir flags.")
.takes_value(true)
.global(true)
)
.arg(
Arg::with_name("env_log")
.short("l")
@ -549,16 +539,9 @@ fn run<E: EthSpec>(
// Allow Prometheus access to the version and commit of the Lighthouse build.
metrics::expose_lighthouse_version();
if matches.is_present("spec") {
warn!(
log,
"The --spec flag is deprecated and will be removed in a future release"
);
}
#[cfg(all(feature = "modern", target_arch = "x86_64"))]
if !std::is_x86_feature_detected!("adx") {
warn!(
slog::warn!(
log,
"CPU seems incompatible with optimized Lighthouse build";
"advice" => "If you get a SIGILL, please try Lighthouse portable build"

View File

@ -243,60 +243,6 @@ fn paranoid_block_proposal_on() {
.with_config(|config| assert!(config.chain.paranoid_block_proposal));
}
#[test]
fn count_unrealized_no_arg() {
CommandLineTest::new()
.flag("count-unrealized", None)
// This flag should be ignored, so there's nothing to test but that the
// client starts with the flag present.
.run_with_zero_port();
}
#[test]
fn count_unrealized_false() {
CommandLineTest::new()
.flag("count-unrealized", Some("false"))
// This flag should be ignored, so there's nothing to test but that the
// client starts with the flag present.
.run_with_zero_port();
}
#[test]
fn count_unrealized_true() {
CommandLineTest::new()
.flag("count-unrealized", Some("true"))
// This flag should be ignored, so there's nothing to test but that the
// client starts with the flag present.
.run_with_zero_port();
}
#[test]
fn count_unrealized_full_no_arg() {
CommandLineTest::new()
.flag("count-unrealized-full", None)
// This flag should be ignored, so there's nothing to test but that the
// client starts with the flag present.
.run_with_zero_port();
}
#[test]
fn count_unrealized_full_false() {
CommandLineTest::new()
.flag("count-unrealized-full", Some("false"))
// This flag should be ignored, so there's nothing to test but that the
// client starts with the flag present.
.run_with_zero_port();
}
#[test]
fn count_unrealized_full_true() {
CommandLineTest::new()
.flag("count-unrealized-full", Some("true"))
// This flag should be ignored, so there's nothing to test but that the
// client starts with the flag present.
.run_with_zero_port();
}
#[test]
fn reset_payload_statuses_default() {
CommandLineTest::new()
@ -388,23 +334,6 @@ fn eth1_flag() {
.with_config(|config| assert!(config.sync_eth1_chain));
}
#[test]
fn eth1_endpoints_flag() {
CommandLineTest::new()
.flag("eth1-endpoints", Some("http://localhost:9545"))
.run_with_zero_port()
.with_config(|config| {
assert_eq!(
config.eth1.endpoint.get_endpoint().full.to_string(),
"http://localhost:9545/"
);
assert_eq!(
config.eth1.endpoint.get_endpoint().to_string(),
"http://localhost:9545/"
);
assert!(config.sync_eth1_chain);
});
}
#[test]
fn eth1_blocks_per_log_query_flag() {
CommandLineTest::new()
.flag("eth1-blocks-per-log-query", Some("500"))
@ -527,49 +456,6 @@ fn merge_execution_endpoints_flag() {
fn merge_execution_endpoint_flag() {
run_merge_execution_endpoints_flag_test("execution-endpoint")
}
fn run_execution_endpoints_overrides_eth1_endpoints_test(eth1_flag: &str, execution_flag: &str) {
use sensitive_url::SensitiveUrl;
let eth1_endpoint = "http://bad.bad";
let execution_endpoint = "http://good.good";
assert!(eth1_endpoint != execution_endpoint);
let dir = TempDir::new().expect("Unable to create temporary directory");
let jwt_path = dir.path().join("jwt-file");
CommandLineTest::new()
.flag(eth1_flag, Some(&eth1_endpoint))
.flag(execution_flag, Some(&execution_endpoint))
.flag("execution-jwt", jwt_path.as_os_str().to_str())
.run_with_zero_port()
.with_config(|config| {
assert_eq!(
config.execution_layer.as_ref().unwrap().execution_endpoints,
vec![SensitiveUrl::parse(execution_endpoint).unwrap()]
);
// The eth1 endpoint should have been set to the --execution-endpoint value in defiance
// of --eth1-endpoints.
assert_eq!(
config.eth1.endpoint,
Eth1Endpoint::Auth {
endpoint: SensitiveUrl::parse(execution_endpoint).unwrap(),
jwt_path: jwt_path.clone(),
jwt_id: None,
jwt_version: None,
}
);
});
}
#[test]
fn execution_endpoints_overrides_eth1_endpoints() {
run_execution_endpoints_overrides_eth1_endpoints_test("eth1-endpoints", "execution-endpoints");
}
#[test]
fn execution_endpoint_overrides_eth1_endpoint() {
run_execution_endpoints_overrides_eth1_endpoints_test("eth1-endpoint", "execution-endpoint");
}
#[test]
fn merge_jwt_secrets_flag() {
let dir = TempDir::new().expect("Unable to create temporary directory");

View File

@ -101,12 +101,6 @@ fn beacon_nodes_flag() {
});
}
#[test]
fn allow_unsynced_flag() {
// No-op, but doesn't crash.
CommandLineTest::new().flag("allow-unsynced", None).run();
}
#[test]
fn disable_auto_discover_flag() {
CommandLineTest::new()

View File

@ -8,15 +8,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
"When connected to a beacon node, performs the duties of a staked \
validator (e.g., proposing blocks and attestations).",
)
// This argument is deprecated, use `--beacon-nodes` instead.
.arg(
Arg::with_name("beacon-node")
.long("beacon-node")
.value_name("NETWORK_ADDRESS")
.help("Deprecated. Use --beacon-nodes.")
.takes_value(true)
.conflicts_with("beacon-nodes"),
)
.arg(
Arg::with_name("beacon-nodes")
.long("beacon-nodes")
@ -45,15 +36,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
api calls only go out to the first available and synced beacon node")
.takes_value(false)
)
// This argument is deprecated, use `--beacon-nodes` instead.
.arg(
Arg::with_name("server")
.long("server")
.value_name("NETWORK_ADDRESS")
.help("Deprecated. Use --beacon-nodes.")
.takes_value(true)
.conflicts_with_all(&["beacon-node", "beacon-nodes"]),
)
.arg(
Arg::with_name("validators-dir")
.long("validators-dir")
@ -80,13 +62,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.takes_value(true)
.conflicts_with("datadir")
)
.arg(
Arg::with_name("delete-lockfiles")
.long("delete-lockfiles")
.help(
"DEPRECATED. This flag does nothing and will be removed in a future release."
)
)
.arg(
Arg::with_name("init-slashing-protection")
.long("init-slashing-protection")
@ -106,11 +81,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
will need to be manually added to the validator_definitions.yml file."
)
)
.arg(
Arg::with_name("allow-unsynced")
.long("allow-unsynced")
.help("DEPRECATED: this flag does nothing"),
)
.arg(
Arg::with_name("use-long-timeouts")
.long("use-long-timeouts")
@ -319,18 +289,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
headers during proposals and will sign over headers. Useful for outsourcing \
execution payload construction during proposals.")
.takes_value(false),
).arg(
Arg::with_name("strict-fee-recipient")
.long("strict-fee-recipient")
.help("[DEPRECATED] If this flag is set, Lighthouse will refuse to sign any block whose \
`fee_recipient` does not match the `suggested_fee_recipient` sent by this validator. \
This applies to both the normal block proposal flow, as well as block proposals \
through the builder API. Proposals through the builder API are more likely to have a \
discrepancy in `fee_recipient` so you should be aware of how your connected relay \
sends proposer payments before using this flag. If this flag is used, a fee recipient \
mismatch in the builder API flow will result in a fallback to the local execution engine \
for payload construction, where a strict fee recipient check will still be applied.")
.takes_value(false),
)
.arg(
Arg::with_name("builder-registration-timestamp-override")

View File

@ -9,7 +9,7 @@ use directory::{
use eth2::types::Graffiti;
use sensitive_url::SensitiveUrl;
use serde::{Deserialize, Serialize};
use slog::{info, warn, Logger};
use slog::{info, Logger};
use std::fs;
use std::net::IpAddr;
use std::path::PathBuf;
@ -171,27 +171,6 @@ impl Config {
.collect::<Result<_, _>>()
.map_err(|e| format!("Unable to parse beacon node URL: {:?}", e))?;
}
// To be deprecated.
else if let Some(beacon_node) = parse_optional::<String>(cli_args, "beacon-node")? {
warn!(
log,
"The --beacon-node flag is deprecated";
"msg" => "please use --beacon-nodes instead"
);
config.beacon_nodes = vec![SensitiveUrl::parse(&beacon_node)
.map_err(|e| format!("Unable to parse beacon node URL: {:?}", e))?];
}
// To be deprecated.
else if let Some(server) = parse_optional::<String>(cli_args, "server")? {
warn!(
log,
"The --server flag is deprecated";
"msg" => "please use --beacon-nodes instead"
);
config.beacon_nodes = vec![SensitiveUrl::parse(&server)
.map_err(|e| format!("Unable to parse beacon node URL: {:?}", e))?];
}
if let Some(proposer_nodes) = parse_optional::<String>(cli_args, "proposer_nodes")? {
config.proposer_nodes = proposer_nodes
.split(',')
@ -200,21 +179,6 @@ impl Config {
.map_err(|e| format!("Unable to parse proposer node URL: {:?}", e))?;
}
if cli_args.is_present("delete-lockfiles") {
warn!(
log,
"The --delete-lockfiles flag is deprecated";
"msg" => "it is no longer necessary, and no longer has any effect",
);
}
if cli_args.is_present("allow-unsynced") {
warn!(
log,
"The --allow-unsynced flag is deprecated";
"msg" => "it no longer has any effect",
);
}
config.disable_run_on_all = cli_args.is_present("disable-run-on-all");
config.disable_auto_discover = cli_args.is_present("disable-auto-discover");
config.init_slashing_protection = cli_args.is_present("init-slashing-protection");
@ -380,14 +344,6 @@ impl Config {
);
}
if cli_args.is_present("strict-fee-recipient") {
warn!(
log,
"The flag `--strict-fee-recipient` has been deprecated due to a bug causing \
missed proposals. The flag will be ignored."
);
}
config.enable_latency_measurement_service =
parse_optional(cli_args, "latency-measurement-service")?.unwrap_or(true);