From 62a2413ade349abeb4ee37de045a94e50ecfb25b Mon Sep 17 00:00:00 2001 From: Gua00va Date: Thu, 8 Jun 2023 13:47:56 +0000 Subject: [PATCH] Enable slasher broadcast by default (#4368) ## Issue Addressed This PR addresses issue https://github.com/sigp/lighthouse/issues/4350 ## Proposed Changes This change will enable slasher broadcast in the following cases: No flag is passed, `--slasher-broadcast` is passed and, `--slasher-broadcast=true` is passed. Only when an explicit false value is passed the slasher does not broadcast.(`--slasher-broadcast=false`). ## Additional Info TODO - [x] Modify CLI parsing logic - [x] Write test Refer to #4353 Co-authored-by: Rahul Dogra Co-authored-by: Gua00va <105484243+Gua00va@users.noreply.github.com> --- beacon_node/src/cli.rs | 5 ++-- beacon_node/src/config.rs | 4 ++- lighthouse/tests/beacon_node.rs | 43 ++++++++++++++++++++++++++++----- 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 10d9ffafd..379eb8e33 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -792,8 +792,9 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { Arg::with_name("slasher-broadcast") .long("slasher-broadcast") .help("Broadcast slashings found by the slasher to the rest of the network \ - [disabled by default].") - .requires("slasher") + [Enabled by default].") + .takes_value(true) + .default_value("true") ) .arg( Arg::with_name("slasher-backend") diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 92e822819..c59b297c1 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -633,7 +633,9 @@ pub fn get_config( slasher_config.validator_chunk_size = validator_chunk_size; } - slasher_config.broadcast = cli_args.is_present("slasher-broadcast"); + if let Some(broadcast) = clap_utils::parse_optional(cli_args, "slasher-broadcast")? { + slasher_config.broadcast = broadcast; + } if let Some(backend) = clap_utils::parse_optional(cli_args, "slasher-backend")? { slasher_config.backend = backend; diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index a71a27bdb..65d7bd08b 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -1912,7 +1912,7 @@ fn slasher_validator_chunk_size_flag() { }); } #[test] -fn slasher_broadcast_flag() { +fn slasher_broadcast_flag_no_args() { CommandLineTest::new() .flag("slasher", None) .flag("slasher-max-db-size", Some("1")) @@ -1926,19 +1926,50 @@ fn slasher_broadcast_flag() { assert!(slasher_config.broadcast); }); } - #[test] -fn slasher_backend_default() { +fn slasher_broadcast_flag_no_default() { CommandLineTest::new() .flag("slasher", None) .flag("slasher-max-db-size", Some("1")) .run_with_zero_port() .with_config(|config| { - let slasher_config = config.slasher.as_ref().unwrap(); - assert_eq!(slasher_config.backend, slasher::DatabaseBackend::Lmdb); + let slasher_config = config + .slasher + .as_ref() + .expect("Unable to parse Slasher config"); + assert!(slasher_config.broadcast); + }); +} +#[test] +fn slasher_broadcast_flag_true() { + CommandLineTest::new() + .flag("slasher", None) + .flag("slasher-max-db-size", Some("1")) + .flag("slasher-broadcast", Some("true")) + .run_with_zero_port() + .with_config(|config| { + let slasher_config = config + .slasher + .as_ref() + .expect("Unable to parse Slasher config"); + assert!(slasher_config.broadcast); + }); +} +#[test] +fn slasher_broadcast_flag_false() { + CommandLineTest::new() + .flag("slasher", None) + .flag("slasher-max-db-size", Some("1")) + .flag("slasher-broadcast", Some("false")) + .run_with_zero_port() + .with_config(|config| { + let slasher_config = config + .slasher + .as_ref() + .expect("Unable to parse Slasher config"); + assert!(!slasher_config.broadcast); }); } - #[test] fn slasher_backend_override_to_default() { // Hard to test this flag because all but one backend is disabled by default and the backend