Correct process_attestation error handling.

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -22,6 +22,7 @@ use store::iter::{BestBlockRootsIterator, BlockIterator, BlockRootsIterator, Sta
 use store::{Error as DBError, Store};
 use tree_hash::TreeHash;
 use types::*;
+use crate::BeaconChainError;
 
 // Text included in blocks.
 // Must be 32-bytes or panic.
@@ -489,15 +490,15 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     pub fn process_attestation(
         &self,
         attestation: Attestation,
-    ) -> Result<(), AttestationValidationError> {
+    ) -> Result<(), Error> {
         self.metrics.attestation_processing_requests.inc();
         let timer = self.metrics.attestation_processing_times.start_timer();
 
         if let Some(state) = self.get_attestation_state(&attestation) {
-            if self.fork_choice.should_process_attestation(&state, &attestation) {
+            if self.fork_choice.should_process_attestation(&state, &attestation)? {
                 let indexed_attestation = common::convert_to_indexed(&state, &attestation)?;
                 per_block_processing::verify_indexed_attestation(&state, &indexed_attestation, &self.spec)?;
-                self.fork_choice.process_attestation(&state, &attestation);
+                self.fork_choice.process_attestation(&state, &attestation)?;
             }
         }
 
@@ -511,7 +512,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             self.metrics.attestation_processing_successes.inc();
         }
 
-        result
+        result.map_err(|e| BeaconChainError::AttestationValidationError(e))
     }
 
     /// Retrieves the `BeaconState` used to create the attestation.
@@ -968,3 +969,4 @@ impl From<BeaconStateError> for Error {
         Error::BeaconStateError(e)
     }
 }
+

beacon_node/beacon_chain/src/errors.rs

@@ -3,6 +3,7 @@ use crate::metrics::Error as MetricsError;
 use state_processing::BlockProcessingError;
 use state_processing::SlotProcessingError;
 use types::*;
+use state_processing::per_block_processing::errors::{AttestationValidationError, IndexedAttestationValidationError};
 
 macro_rules! easy_from_to {
     ($from: ident, $to: ident) => {
@@ -31,6 +32,8 @@ pub enum BeaconChainError {
     MissingBeaconState(Hash256),
     SlotProcessingError(SlotProcessingError),
     MetricsError(String),
+    AttestationValidationError(AttestationValidationError),
+    IndexedAttestationValidationError(IndexedAttestationValidationError)
 }
 
 easy_from_to!(SlotProcessingError, BeaconChainError);
@@ -53,3 +56,5 @@ pub enum BlockProductionError {
 easy_from_to!(BlockProcessingError, BlockProductionError);
 easy_from_to!(BeaconStateError, BlockProductionError);
 easy_from_to!(SlotProcessingError, BlockProductionError);
+easy_from_to!(AttestationValidationError, BeaconChainError);
+easy_from_to!(IndexedAttestationValidationError, BeaconChainError);

beacon_node/beacon_chain/src/fork_choice.rs

@@ -166,24 +166,24 @@ impl<T: BeaconChainTypes> ForkChoice<T> {
         Ok(())
     }
 
-    /// Determines whether or not the given attestation contains a latest messages.
+    /// Determines whether or not the given attestation contains a latest message.
-    pub fn should_process_attestation(&self, state: &BeaconState<T::EthSpec>, attestation: &Attestation) -> bool {
+    pub fn should_process_attestation(&self, state: &BeaconState<T::EthSpec>, attestation: &Attestation) -> Result<bool> {
         let validator_indices = common::get_attesting_indices_unsorted(
             state,
             &attestation.data,
             &attestation.aggregation_bitfield,
-        ).unwrap();
+        )?;
 
         let target_slot = attestation.data.target_epoch.start_slot(T::EthSpec::slots_per_epoch());
 
-        validator_indices
+        Ok(validator_indices
             .iter()
             .find(|&&v| {
                 match self.backend.latest_message(v) {
                     Some((_, slot)) => target_slot > slot,
                     None => true
                 }
-            }).is_some()
+            }).is_some())
     }
 
     /// Inform the fork choice that the given block (and corresponding root) have been finalized so
@@ -218,3 +218,4 @@ impl From<String> for Error {
         Error::BackendError(e)
     }
 }
+

beacon_node/rpc/src/attestation.rs

@@ -1,4 +1,4 @@
-use beacon_chain::{BeaconChain, BeaconChainTypes};
+use beacon_chain::{BeaconChain, BeaconChainTypes, BeaconChainError};
 use eth2_libp2p::PubsubMessage;
 use eth2_libp2p::TopicBuilder;
 use eth2_libp2p::BEACON_ATTESTATION_TOPIC;
@@ -159,7 +159,7 @@ impl<T: BeaconChainTypes> AttestationService for AttestationServiceInstance<T> {
 
                 resp.set_success(true);
             }
-            Err(e) => {
+            Err(BeaconChainError::AttestationValidationError(e)) => {
                 // Attestation was invalid
                 warn!(
                     self.log,
@@ -170,6 +170,28 @@ impl<T: BeaconChainTypes> AttestationService for AttestationServiceInstance<T> {
                 resp.set_success(false);
                 resp.set_msg(format!("InvalidAttestation: {:?}", e).as_bytes().to_vec());
             }
+            Err(BeaconChainError::IndexedAttestationValidationError(e)) => {
+                // Indexed attestation was invalid
+                warn!(
+                    self.log,
+                    "PublishAttestation";
+                    "type" => "invalid_attestation",
+                    "error" => format!("{:?}", e),
+                );
+                resp.set_success(false);
+                resp.set_msg(format!("InvalidIndexedAttestation: {:?}", e).as_bytes().to_vec());
+            }
+            Err(e) => {
+                // Attestation was invalid
+                warn!(
+                    self.log,
+                    "PublishAttestation";
+                    "type" => "beacon_chain_error",
+                    "error" => format!("{:?}", e),
+                );
+                resp.set_success(false);
+                resp.set_msg(format!("There was a beacon chain error: {:?}", e).as_bytes().to_vec());
+            }
         };
 
         let error_log = self.log.clone();