From 6ad2c187ddae833c47927211db6b447566f6679f Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Fri, 18 Oct 2024 15:21:46 +1100 Subject: [PATCH] Remove deprecated flags in prep for v6.0.0 (#6490) * Delete previously deprecated flags * Update CLI docs * Remove deprecated BN flags * Use ethereum-package main branch * Delete env_log/-l --- beacon_node/http_api/src/lib.rs | 5 +- .../operation_pool/src/attestation_id.rs | 12 ---- beacon_node/operation_pool/src/lib.rs | 1 - beacon_node/src/cli.rs | 61 ------------------- beacon_node/src/config.rs | 40 ------------ book/src/help_bn.md | 9 --- book/src/help_general.md | 3 - book/src/help_vc.md | 8 --- book/src/redundancy.md | 3 +- consensus/types/src/config_and_preset.rs | 1 + lighthouse/src/main.rs | 10 --- lighthouse/tests/beacon_node.rs | 52 ---------------- lighthouse/tests/validator_client.rs | 34 ----------- scripts/local_testnet/start_local_testnet.sh | 2 +- validator_client/src/cli.rs | 31 ---------- validator_client/src/config.rs | 27 -------- 16 files changed, 4 insertions(+), 295 deletions(-) delete mode 100644 beacon_node/operation_pool/src/attestation_id.rs diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index ffcfda46803..307584b82d4 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -146,7 +146,6 @@ pub struct Config { pub listen_port: u16, pub allow_origin: Option, pub tls_config: Option, - pub spec_fork_name: Option, pub data_dir: PathBuf, pub sse_capacity_multiplier: usize, pub enable_beacon_processor: bool, @@ -164,7 +163,6 @@ impl Default for Config { listen_port: 5052, allow_origin: None, tls_config: None, - spec_fork_name: None, data_dir: PathBuf::from(DEFAULT_ROOT_DIR), sse_capacity_multiplier: 1, enable_beacon_processor: true, @@ -2643,7 +2641,6 @@ pub fn serve( ); // GET config/spec - let spec_fork_name = ctx.config.spec_fork_name; let get_config_spec = config_path .and(warp::path("spec")) .and(warp::path::end()) @@ -2653,7 +2650,7 @@ pub fn serve( move |task_spawner: TaskSpawner, chain: Arc>| { task_spawner.blocking_json_task(Priority::P0, move || { let config_and_preset = - ConfigAndPreset::from_chain_spec::(&chain.spec, spec_fork_name); + ConfigAndPreset::from_chain_spec::(&chain.spec, None); Ok(api_types::GenericResponse::from(config_and_preset)) }) }, diff --git a/beacon_node/operation_pool/src/attestation_id.rs b/beacon_node/operation_pool/src/attestation_id.rs deleted file mode 100644 index f0dc6536a54..00000000000 --- a/beacon_node/operation_pool/src/attestation_id.rs +++ /dev/null @@ -1,12 +0,0 @@ -use serde::{Deserialize, Serialize}; -use ssz_derive::{Decode, Encode}; - -/// Serialized `AttestationData` augmented with a domain to encode the fork info. -/// -/// [DEPRECATED] To be removed once all nodes have updated to schema v12. -#[derive( - PartialEq, Eq, Clone, Hash, Debug, PartialOrd, Ord, Encode, Decode, Serialize, Deserialize, -)] -pub struct AttestationId { - v: Vec, -} diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index 0b032b0c8a7..3a002bf8703 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -1,5 +1,4 @@ mod attestation; -mod attestation_id; mod attestation_storage; mod attester_slashing; mod bls_to_execution_changes; diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index d6ed1068036..dff030fb0f1 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -401,15 +401,6 @@ pub fn cli_app() -> Command { .help_heading(FLAG_HEADER) .display_order(0) ) - .arg( - Arg::new("self-limiter") - .long("self-limiter") - .help("This flag is deprecated and has no effect.") - .hide(true) - .action(ArgAction::SetTrue) - .help_heading(FLAG_HEADER) - .display_order(0) - ) .arg( Arg::new("disable-self-limiter") .long("disable-self-limiter") @@ -525,16 +516,6 @@ pub fn cli_app() -> Command { .action(ArgAction::Set) .display_order(0) ) - .arg( - Arg::new("http-spec-fork") - .long("http-spec-fork") - .requires("enable_http") - .value_name("FORK") - .help("This flag is deprecated and has no effect.") - .hide(true) - .action(ArgAction::Set) - .display_order(0) - ) .arg( Arg::new("http-enable-tls") .long("http-enable-tls") @@ -564,16 +545,6 @@ pub fn cli_app() -> Command { .action(ArgAction::Set) .display_order(0) ) - .arg( - Arg::new("http-allow-sync-stalled") - .long("http-allow-sync-stalled") - .action(ArgAction::SetTrue) - .help_heading(FLAG_HEADER) - .requires("enable_http") - .help("This flag is deprecated and has no effect.") - .hide(true) - .display_order(0) - ) .arg( Arg::new("http-sse-capacity-multiplier") .long("http-sse-capacity-multiplier") @@ -1291,14 +1262,6 @@ pub fn cli_app() -> Command { .action(ArgAction::Set) .display_order(0) ) - .arg( - Arg::new("disable-lock-timeouts") - .long("disable-lock-timeouts") - .help("This flag is deprecated and has no effect.") - .action(ArgAction::SetTrue) - .help_heading(FLAG_HEADER) - .display_order(0) - ) .arg( Arg::new("disable-proposer-reorgs") .long("disable-proposer-reorgs") @@ -1511,14 +1474,6 @@ pub fn cli_app() -> Command { .help_heading(FLAG_HEADER) .display_order(0) ) - .arg( - Arg::new("always-prefer-builder-payload") - .action(ArgAction::SetTrue) - .help_heading(FLAG_HEADER) - .long("always-prefer-builder-payload") - .help("This flag is deprecated and has no effect.") - .display_order(0) - ) .arg( Arg::new("invalid-gossip-verified-blocks-path") .action(ArgAction::Set) @@ -1530,14 +1485,6 @@ pub fn cli_app() -> Command { filling up their disks.") .display_order(0) ) - .arg( - Arg::new("progressive-balances") - .long("progressive-balances") - .value_name("MODE") - .help("Deprecated. This optimisation is now the default and cannot be disabled.") - .action(ArgAction::Set) - .display_order(0) - ) .arg( Arg::new("beacon-processor-max-workers") .long("beacon-processor-max-workers") @@ -1599,13 +1546,5 @@ pub fn cli_app() -> Command { .action(ArgAction::Set) .display_order(0) ) - .arg( - Arg::new("disable-duplicate-warn-logs") - .long("disable-duplicate-warn-logs") - .help("This flag is deprecated and has no effect.") - .action(ArgAction::SetTrue) - .help_heading(FLAG_HEADER) - .display_order(0) - ) .group(ArgGroup::new("enable_http").args(["http", "gui", "staking"]).multiple(true)) } diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index f62ccfe3ed9..2d318153515 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -152,14 +152,6 @@ pub fn get_config( client_config.http_api.allow_origin = Some(allow_origin.to_string()); } - if cli_args.get_one::("http-spec-fork").is_some() { - warn!( - log, - "Ignoring --http-spec-fork"; - "info" => "this flag is deprecated and will be removed" - ); - } - if cli_args.get_flag("http-enable-tls") { client_config.http_api.tls_config = Some(TlsConfig { cert: cli_args @@ -175,14 +167,6 @@ pub fn get_config( }); } - if cli_args.get_flag("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")?; @@ -362,14 +346,6 @@ pub fn get_config( .map(Duration::from_millis); } - if cli_args.get_flag("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_file = Some(secret_file.clone()); el_config.execution_endpoint = Some(execution_endpoint.clone()); @@ -787,14 +763,6 @@ pub fn get_config( .individual_tracking_threshold = count; } - if cli_args.get_flag("disable-lock-timeouts") { - warn!( - log, - "Ignoring --disable-lock-timeouts"; - "info" => "this flag is deprecated and will be removed" - ); - } - if cli_args.get_flag("disable-proposer-reorgs") { client_config.chain.re_org_head_threshold = None; client_config.chain.re_org_parent_threshold = None; @@ -894,14 +862,6 @@ pub fn get_config( client_config.network.invalid_block_storage = Some(path); } - if cli_args.get_one::("progressive-balances").is_some() { - warn!( - log, - "Progressive balances mode is deprecated"; - "info" => "please remove --progressive-balances" - ); - } - if let Some(max_workers) = clap_utils::parse_optional(cli_args, "beacon-processor-max-workers")? { client_config.beacon_processor.max_workers = max_workers; diff --git a/book/src/help_bn.md b/book/src/help_bn.md index 733446e5d27..338905a4fbf 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -292,9 +292,6 @@ Options: which don't improve their payload after the first call, and high values are useful for ensuring the EL is given ample notice. Default: 1/3 of a slot. - --progressive-balances - Deprecated. This optimisation is now the default and cannot be - disabled. --proposer-reorg-cutoff Maximum delay after the start of the slot at which to propose a reorging block. Lower values can prevent failed reorgs by ensuring the @@ -445,8 +442,6 @@ Flags: incompatible with data availability checks. Checkpoint syncing is the preferred method for syncing a node. Only use this flag when testing. DO NOT use on mainnet! - --always-prefer-builder-payload - This flag is deprecated and has no effect. --always-prepare-payload Send payload attributes with every fork choice update. This is intended for use by block builders, relays and developers. You should @@ -470,8 +465,6 @@ Flags: Explicitly disables syncing of deposit logs from the execution node. This overrides any previous option that depends on it. Useful if you intend to run a non-validating beacon node. - --disable-duplicate-warn-logs - This flag is deprecated and has no effect. --disable-enr-auto-update Discovery automatically updates the nodes local ENR with an external IP address and port as seen by other peers on the network. This @@ -479,8 +472,6 @@ Flags: boot. --disable-inbound-rate-limiter Disables the inbound rate limiter (requests received by this node). - --disable-lock-timeouts - This flag is deprecated and has no effect. --disable-log-timestamp If present, do not include timestamps in logging output. --disable-malloc-tuning diff --git a/book/src/help_general.md b/book/src/help_general.md index 1c2d1266d08..48314d5108c 100644 --- a/book/src/help_general.md +++ b/book/src/help_general.md @@ -122,9 +122,6 @@ Flags: debugging specific memory allocation issues. -h, --help Prints help information - -l - DEPRECATED Enables environment logging giving access to sub-protocol - logs such as discv5 and libp2p --log-color Force outputting colors when emitting logs to the terminal. --logfile-compress diff --git a/book/src/help_vc.md b/book/src/help_vc.md index 23a84919936..aa24ab3d91f 100644 --- a/book/src/help_vc.md +++ b/book/src/help_vc.md @@ -210,12 +210,6 @@ Flags: If present, do not configure the system allocator. Providing this flag will generally increase memory usage, it should only be provided when debugging specific memory allocation issues. - --disable-run-on-all - DEPRECATED. Use --broadcast. By default, Lighthouse publishes - attestation, sync committee subscriptions and proposer preparation - messages to all beacon nodes provided in the `--beacon-nodes flag`. - This option changes that behaviour such that these api calls only go - out to the first available and synced beacon node --disable-slashing-protection-web3signer Disable Lighthouse's slashing protection for all web3signer keys. This can reduce the I/O burden on the VC but is only safe if slashing @@ -280,8 +274,6 @@ Flags: --prefer-builder-proposals If this flag is set, Lighthouse will always prefer blocks constructed by builders, regardless of payload value. - --produce-block-v3 - This flag is deprecated and is no longer in use. --stdin-inputs If present, read all user inputs from stdin instead of tty. --unencrypted-http-transport diff --git a/book/src/redundancy.md b/book/src/redundancy.md index ee685a17cf7..daf0eb4a5b4 100644 --- a/book/src/redundancy.md +++ b/book/src/redundancy.md @@ -74,8 +74,7 @@ lighthouse bn \ Prior to v3.2.0 fallback beacon nodes also required the `--subscribe-all-subnets` and `--import-all-attestations` flags. These flags are no longer required as the validator client will now broadcast subscriptions to all connected beacon nodes by default. This broadcast behaviour -can be disabled using the `--broadcast none` flag for `lighthouse vc` (or `--disable-run-on-all` -[deprecated]). +can be disabled using the `--broadcast none` flag for `lighthouse vc`. ### Broadcast modes diff --git a/consensus/types/src/config_and_preset.rs b/consensus/types/src/config_and_preset.rs index b1e9049b0df..c80d678b2a3 100644 --- a/consensus/types/src/config_and_preset.rs +++ b/consensus/types/src/config_and_preset.rs @@ -41,6 +41,7 @@ pub struct ConfigAndPreset { } impl ConfigAndPreset { + // DEPRECATED: the `fork_name` argument is never used, we should remove it. pub fn from_chain_spec(spec: &ChainSpec, fork_name: Option) -> Self { let config = Config::from_chain_spec::(spec); let base_preset = BasePreset::from_chain_spec::(spec); diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index aad8860fccb..4f4dabff89b 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -115,16 +115,6 @@ fn main() { .global(true) .display_order(0), ) - .arg( - Arg::new("env_log") - .short('l') - .help( - "DEPRECATED Enables environment logging giving access to sub-protocol logs such as discv5 and libp2p", - ) - .action(ArgAction::SetTrue) - .help_heading(FLAG_HEADER) - .display_order(0) - ) .arg( Arg::new("logfile") .long("logfile") diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index f3832a1a1e5..f22e4387008 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -159,13 +159,6 @@ fn max_skip_slots_flag() { .with_config(|config| assert_eq!(config.chain.import_max_skip_slots, Some(10))); } -#[test] -fn disable_lock_timeouts_flag() { - CommandLineTest::new() - .flag("disable-lock-timeouts", None) - .run_with_zero_port(); -} - #[test] fn shuffling_cache_default() { CommandLineTest::new() @@ -1612,19 +1605,6 @@ fn http_port_flag() { .run() .with_config(|config| assert_eq!(config.http_api.listen_port, port1)); } -#[test] -fn empty_self_limiter_flag() { - // Test that empty rate limiter is accepted using the default rate limiting configurations. - CommandLineTest::new() - .flag("self-limiter", None) - .run_with_zero_port() - .with_config(|config| { - assert_eq!( - config.network.outbound_rate_limiter_config, - Some(lighthouse_network::rpc::config::OutboundRateLimiterConfig::default()) - ) - }); -} #[test] fn empty_inbound_rate_limiter_flag() { @@ -1667,14 +1647,6 @@ 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() @@ -1713,22 +1685,6 @@ fn http_tls_flags() { }); } -#[test] -fn http_spec_fork_default() { - CommandLineTest::new() - .flag("http", None) - .run_with_zero_port() - .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() { @@ -2631,14 +2587,6 @@ fn invalid_gossip_verified_blocks_path() { }); } -#[test] -fn progressive_balances_checked() { - // Flag is deprecated but supplying it should not crash until we remove it completely. - CommandLineTest::new() - .flag("progressive-balances", Some("checked")) - .run_with_zero_port(); -} - #[test] fn beacon_processor() { CommandLineTest::new() diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index baf50aa7c07..147a371f0eb 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -426,13 +426,6 @@ fn no_doppelganger_protection_flag() { .with_config(|config| assert!(!config.enable_doppelganger_protection)); } -#[test] -fn produce_block_v3_flag() { - // The flag is DEPRECATED but providing it should not trigger an error. - // We can delete this test when deleting the flag entirely. - CommandLineTest::new().flag("produce-block-v3", None).run(); -} - #[test] fn no_gas_limit_flag() { CommandLineTest::new() @@ -513,23 +506,6 @@ fn monitoring_endpoint() { assert_eq!(api_conf.update_period_secs, Some(30)); }); } -#[test] -fn disable_run_on_all_flag() { - CommandLineTest::new() - .flag("disable-run-on-all", None) - .run() - .with_config(|config| { - assert_eq!(config.broadcast_topics, vec![]); - }); - // --broadcast flag takes precedence - CommandLineTest::new() - .flag("disable-run-on-all", None) - .flag("broadcast", Some("attestations")) - .run() - .with_config(|config| { - assert_eq!(config.broadcast_topics, vec![ApiTopic::Attestations]); - }); -} #[test] fn no_broadcast_flag() { @@ -623,16 +599,6 @@ fn disable_latency_measurement_service() { assert!(!config.enable_latency_measurement_service); }); } -#[test] -fn latency_measurement_service() { - // This flag is DEPRECATED so has no effect, but should still be accepted. - CommandLineTest::new() - .flag("latency-measurement-service", Some("false")) - .run() - .with_config(|config| { - assert!(config.enable_latency_measurement_service); - }); -} #[test] fn validator_registration_batch_size() { diff --git a/scripts/local_testnet/start_local_testnet.sh b/scripts/local_testnet/start_local_testnet.sh index f90132764e4..1f156886931 100755 --- a/scripts/local_testnet/start_local_testnet.sh +++ b/scripts/local_testnet/start_local_testnet.sh @@ -7,7 +7,7 @@ set -Eeuo pipefail SCRIPT_DIR="$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" ENCLAVE_NAME=local-testnet NETWORK_PARAMS_FILE=$SCRIPT_DIR/network_params.yaml -ETHEREUM_PKG_VERSION=4.2.0 +ETHEREUM_PKG_VERSION=main BUILD_IMAGE=true BUILDER_PROPOSALS=false diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index b027ad0df6d..209876f07b0 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -39,20 +39,6 @@ pub fn cli_app() -> Command { .action(ArgAction::Set) .display_order(0) ) - // TODO remove this flag in a future release - .arg( - Arg::new("disable-run-on-all") - .long("disable-run-on-all") - .value_name("DISABLE_RUN_ON_ALL") - .help("DEPRECATED. Use --broadcast. \ - By default, Lighthouse publishes attestation, sync committee subscriptions \ - and proposer preparation messages to all beacon nodes provided in the \ - `--beacon-nodes flag`. This option changes that behaviour such that these \ - api calls only go out to the first available and synced beacon node") - .action(ArgAction::SetTrue) - .help_heading(FLAG_HEADER) - .display_order(0) - ) .arg( Arg::new("broadcast") .long("broadcast") @@ -167,14 +153,6 @@ pub fn cli_app() -> Command { .action(ArgAction::Set) .display_order(0) ) - .arg( - Arg::new("produce-block-v3") - .long("produce-block-v3") - .help("This flag is deprecated and is no longer in use.") - .action(ArgAction::SetTrue) - .help_heading(FLAG_HEADER) - .display_order(0) - ) .arg( Arg::new("distributed") .long("distributed") @@ -403,15 +381,6 @@ pub fn cli_app() -> Command { .help_heading(FLAG_HEADER) .display_order(0) ) - .arg( - Arg::new("latency-measurement-service") - .long("latency-measurement-service") - .help("DEPRECATED") - .action(ArgAction::Set) - .help_heading(FLAG_HEADER) - .display_order(0) - .hide(true) - ) .arg( Arg::new("validator-registration-batch-size") .long("validator-registration-batch-size") diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index c2c445c48c3..f42ed551463 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -244,14 +244,6 @@ impl Config { config.distributed = true; } - if cli_args.get_flag("disable-run-on-all") { - warn!( - log, - "The --disable-run-on-all flag is deprecated"; - "msg" => "please use --broadcast instead" - ); - config.broadcast_topics = vec![]; - } if let Some(broadcast_topics) = cli_args.get_one::("broadcast") { config.broadcast_topics = broadcast_topics .split(',') @@ -397,14 +389,6 @@ impl Config { config.prefer_builder_proposals = true; } - if cli_args.get_flag("produce-block-v3") { - warn!( - log, - "produce-block-v3 flag"; - "note" => "deprecated flag has no effect and should be removed" - ); - } - config.gas_limit = cli_args .get_one::("gas-limit") .map(|gas_limit| { @@ -429,17 +413,6 @@ impl Config { config.enable_latency_measurement_service = !cli_args.get_flag("disable-latency-measurement-service"); - if cli_args - .get_one::("latency-measurement-service") - .is_some() - { - warn!( - log, - "latency-measurement-service flag"; - "note" => "deprecated flag has no effect and should be removed" - ); - } - config.validator_registration_batch_size = parse_required(cli_args, "validator-registration-batch-size")?; if config.validator_registration_batch_size == 0 {