Enable slasher broadcast by default (sigp#4368)

## Issue Addressed This PR addresses issue sigp#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 sigp#4353 Co-authored-by: Rahul Dogra <[email protected]> Co-authored-by: Gua00va <[email protected]>
Woodpile37 · Jan 6, 2024 · 2b380f0 · 2b380f0
1 parent 184a2ca
commit 2b380f0
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 9 deletions.
diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs
@@ -834,8 +834,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
@@ -670,7 +670,9 @@ pub fn get_config<E: EthSpec>(
             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
@@ -1951,7 +1951,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"))
@@ -1965,19 +1965,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