Add gossip validation spec tests for proposer/attester slashings

beacon_node/network/src/lib.rs

@@ -11,6 +11,7 @@ mod subnet_service;
 mod sync;
 
 pub use lighthouse_network::NetworkConfig;
+pub use network_beacon_processor::NetworkBeaconProcessor;
 pub use service::{
     NetworkMessage, NetworkReceivers, NetworkSenders, NetworkService, ValidatorSubscriptionMessage,
 };

beacon_node/network/src/network_beacon_processor/gossip_methods.rs

@@ -174,6 +174,15 @@ impl<E: EthSpec> FailedAtt<E> {
     }
 }
 
+/// `MessageAcceptance` doesn't implement clone so we do a manual match here.
+fn clone_message_acceptance(a: &MessageAcceptance) -> MessageAcceptance {
+    match a {
+        MessageAcceptance::Accept => MessageAcceptance::Accept,
+        MessageAcceptance::Reject => MessageAcceptance::Reject,
+        MessageAcceptance::Ignore => MessageAcceptance::Ignore,
+    }
+}
+
 impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
     /* Auxiliary functions */
 
@@ -2190,23 +2199,22 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
         message_id: MessageId,
         peer_id: PeerId,
         proposer_slashing: ProposerSlashing,
-    ) {
+    ) -> MessageAcceptance {
         let validator_index = proposer_slashing.signed_header_1.message.proposer_index;
 
-        let slashing = match self
+        let (validation_result, verified_slashing_opt) = match self
             .chain
             .verify_proposer_slashing_for_gossip(proposer_slashing)
         {
-            Ok(ObservationOutcome::New(slashing)) => slashing,
+            Ok(ObservationOutcome::New(slashing)) => (MessageAcceptance::Accept, Some(slashing)),
             Ok(ObservationOutcome::AlreadyKnown) => {
                 debug!(
                     reason = "Already seen a proposer slashing for that validator",
                     validator_index,
                     peer = %peer_id,
                     "Dropping proposer slashing"
                 );
-                self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore);
-                return;
+                (MessageAcceptance::Ignore, None)
             }
             Err(e) => {
                 // This is likely a fault with the beacon chain and not necessarily a
@@ -2217,84 +2225,96 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                     error = ?e,
                     "Dropping invalid proposer slashing"
                 );
-                self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore);
 
                 // Penalize peer slightly for invalids.
                 self.gossip_penalize_peer(
                     peer_id,
                     PeerAction::HighToleranceError,
                     "invalid_gossip_proposer_slashing",
                 );
-                return;
+                (MessageAcceptance::Ignore, None)
             }
         };
 
-        metrics::inc_counter(&metrics::BEACON_PROCESSOR_PROPOSER_SLASHING_VERIFIED_TOTAL);
+        self.propagate_validation_result(
+            message_id,
+            peer_id,
+            clone_message_acceptance(&validation_result),
+        );
 
-        self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Accept);
+        if let Some(slashing) = verified_slashing_opt {
+            metrics::inc_counter(&metrics::BEACON_PROCESSOR_PROPOSER_SLASHING_VERIFIED_TOTAL);
 
-        // Register the slashing with any monitored validators.
-        self.chain
-            .validator_monitor
-            .read()
-            .register_gossip_proposer_slashing(slashing.as_inner());
+            // Register the slashing with any monitored validators.
+            self.chain
+                .validator_monitor
+                .read()
+                .register_gossip_proposer_slashing(slashing.as_inner());
 
-        self.chain.import_proposer_slashing(slashing);
-        debug!("Successfully imported proposer slashing");
+            self.chain.import_proposer_slashing(slashing);
+            debug!("Successfully imported proposer slashing");
 
-        metrics::inc_counter(&metrics::BEACON_PROCESSOR_PROPOSER_SLASHING_IMPORTED_TOTAL);
+            metrics::inc_counter(&metrics::BEACON_PROCESSOR_PROPOSER_SLASHING_IMPORTED_TOTAL);
+        }
+
+        validation_result
     }
 
     pub fn process_gossip_attester_slashing(
         self: &Arc<Self>,
         message_id: MessageId,
         peer_id: PeerId,
         attester_slashing: AttesterSlashing<T::EthSpec>,
-    ) {
-        let slashing = match self
+    ) -> MessageAcceptance {
+        let (validation_result, verified_slashing_opt) = match self
             .chain
             .verify_attester_slashing_for_gossip(attester_slashing)
         {
-            Ok(ObservationOutcome::New(slashing)) => slashing,
+            Ok(ObservationOutcome::New(slashing)) => (MessageAcceptance::Accept, Some(slashing)),
             Ok(ObservationOutcome::AlreadyKnown) => {
                 debug!(
                     reason = "Slashings already known for all slashed validators",
                     peer = %peer_id,
                     "Dropping attester slashing"
                 );
-                self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore);
-                return;
+                (MessageAcceptance::Ignore, None)
             }
             Err(e) => {
                 debug!(
                     %peer_id,
                     error = ?e,
                     "Dropping invalid attester slashing"
                 );
-                self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore);
                 // Penalize peer slightly for invalids.
                 self.gossip_penalize_peer(
                     peer_id,
                     PeerAction::HighToleranceError,
                     "invalid_gossip_attester_slashing",
                 );
-                return;
+                (MessageAcceptance::Ignore, None)
             }
         };
 
-        metrics::inc_counter(&metrics::BEACON_PROCESSOR_ATTESTER_SLASHING_VERIFIED_TOTAL);
+        self.propagate_validation_result(
+            message_id,
+            peer_id,
+            clone_message_acceptance(&validation_result),
+        );
 
-        self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Accept);
+        if let Some(slashing) = verified_slashing_opt {
+            metrics::inc_counter(&metrics::BEACON_PROCESSOR_ATTESTER_SLASHING_VERIFIED_TOTAL);
 
-        // Register the slashing with any monitored validators.
-        self.chain
-            .validator_monitor
-            .read()
-            .register_gossip_attester_slashing(slashing.as_inner().to_ref());
+            self.chain
+                .validator_monitor
+                .read()
+                .register_gossip_attester_slashing(slashing.as_inner().to_ref());
+
+            self.chain.import_attester_slashing(slashing);
+            debug!("Successfully imported attester slashing");
+            metrics::inc_counter(&metrics::BEACON_PROCESSOR_ATTESTER_SLASHING_IMPORTED_TOTAL);
+        }
 
-        self.chain.import_attester_slashing(slashing);
-        debug!("Successfully imported attester slashing");
-        metrics::inc_counter(&metrics::BEACON_PROCESSOR_ATTESTER_SLASHING_IMPORTED_TOTAL);
+        validation_result
     }
 
     pub fn process_gossip_bls_to_execution_change(

beacon_node/network/src/network_beacon_processor/mod.rs

@@ -20,7 +20,7 @@ use lighthouse_network::rpc::methods::{
 };
 use lighthouse_network::service::api_types::CustodyBackfillBatchId;
 use lighthouse_network::{
-    Client, GossipTopic, MessageId, NetworkGlobals, PeerId, PubsubMessage,
+    Client, GossipTopic, MessageId, NetworkConfig, NetworkGlobals, PeerId, PubsubMessage,
     rpc::{BlocksByRangeRequest, BlocksByRootRequest, LightClientBootstrapRequest, StatusMessage},
 };
 use rand::prelude::SliceRandom;
@@ -353,7 +353,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
     ) -> Result<(), Error<T::EthSpec>> {
         let processor = self.clone();
         let process_fn = move || {
-            processor.process_gossip_proposer_slashing(message_id, peer_id, *proposer_slashing)
+            processor.process_gossip_proposer_slashing(message_id, peer_id, *proposer_slashing);
         };
 
         self.try_send(BeaconWorkEvent {
@@ -420,7 +420,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
     ) -> Result<(), Error<T::EthSpec>> {
         let processor = self.clone();
         let process_fn = move || {
-            processor.process_gossip_attester_slashing(message_id, peer_id, *attester_slashing)
+            processor.process_gossip_attester_slashing(message_id, peer_id, *attester_slashing);
         };
 
         self.try_send(BeaconWorkEvent {
@@ -1241,17 +1241,15 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
     }
 }
 
-#[cfg(test)]
+use beacon_chain::test_utils::{BaseHarnessType, BeaconChainHarness};
 use {
     beacon_chain::builder::Witness, beacon_processor::BeaconProcessorChannels,
     slot_clock::ManualSlotClock, store::MemoryStore, tokio::sync::mpsc::UnboundedSender,
 };
 
-#[cfg(test)]
 pub(crate) type TestBeaconChainType<E> =
     Witness<ManualSlotClock, E, MemoryStore<E>, MemoryStore<E>>;
 
-#[cfg(test)]
 impl<E: EthSpec> NetworkBeaconProcessor<TestBeaconChainType<E>> {
     // Instantiates a mostly non-functional version of `Self` and returns the
     // event receiver that would normally go to the beacon processor. This is
@@ -1283,4 +1281,23 @@ impl<E: EthSpec> NetworkBeaconProcessor<TestBeaconChainType<E>> {
 
         (network_beacon_processor, beacon_processor_rx)
     }
+
+    // TODO: add comment
+    pub fn null_from_harness(
+        harness: &BeaconChainHarness<BaseHarnessType<E, MemoryStore<E>, MemoryStore<E>>>,
+    ) -> Self {
+        let network_globals = NetworkGlobals::new_test_globals(
+            vec![],
+            Arc::new(NetworkConfig::default()),
+            harness.spec.clone(),
+        );
+
+        Self::null_for_testing(
+            Arc::new(network_globals),
+            mpsc::unbounded_channel().0,
+            harness.chain.clone(),
+            harness.runtime.task_executor.clone(),
+        )
+        .0
+    }
 }

testing/ef_tests/Cargo.toml

@@ -26,6 +26,8 @@ fork_choice = { workspace = true }
 fs2 = { workspace = true }
 hex = { workspace = true }
 kzg = { workspace = true }
+lighthouse_network = { workspace = true }
+network = { workspace = true }
 logging = { workspace = true }
 milhouse = { workspace = true }
 rayon = { workspace = true }

testing/ef_tests/check_all_files_accessed.py

@@ -75,8 +75,14 @@
     "tests/.*/compute_challenge/.*",
     # We don't need these manifest files at the moment.
     "tests/.*/manifest.yaml",
-    # TODO: gossip condition tests not implemented yet
-    "tests/.*/.*/networking/.*",
+    # TODO: Remaining gossip validation topics not yet implemented
+    "tests/.*/.*/networking/gossip_beacon_block/.*",
+    "tests/.*/.*/networking/gossip_beacon_attestation/.*",
+    "tests/.*/.*/networking/gossip_beacon_aggregate_and_proof/.*",
+    "tests/.*/.*/networking/gossip_voluntary_exit/.*",
+    "tests/.*/.*/networking/gossip_bls_to_execution_change/.*",
+    "tests/.*/.*/networking/gossip_sync_committee_message/.*",
+    "tests/.*/.*/networking/gossip_sync_committee_contribution_and_proof/.*",
     # TODO: fast confirmation rule not merged yet
     "tests/.*/.*/fast_confirmation",
 ]

testing/ef_tests/src/cases.rs

@@ -20,6 +20,7 @@ mod fork_choice;
 mod genesis_initialization;
 mod genesis_validity;
 mod get_custody_groups;
+mod gossip_validation;
 mod kzg_blob_to_kzg_commitment;
 mod kzg_compute_blob_kzg_proof;
 mod kzg_compute_cells;
@@ -57,6 +58,7 @@ pub use fork::ForkTest;
 pub use genesis_initialization::*;
 pub use genesis_validity::*;
 pub use get_custody_groups::*;
+pub use gossip_validation::*;
 pub use kzg_blob_to_kzg_commitment::*;
 pub use kzg_compute_blob_kzg_proof::*;
 pub use kzg_compute_cells::*;