diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index f215465f2f..67d9281277 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/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 BeaconChain { 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 BeaconChain { 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 for Error { Error::BeaconStateError(e) } } + diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index 0d619d7f2d..4e2170ca84 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/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); diff --git a/beacon_node/beacon_chain/src/fork_choice.rs b/beacon_node/beacon_chain/src/fork_choice.rs index 6b69e3e084..92b683590a 100644 --- a/beacon_node/beacon_chain/src/fork_choice.rs +++ b/beacon_node/beacon_chain/src/fork_choice.rs @@ -166,24 +166,24 @@ impl ForkChoice { Ok(()) } - /// Determines whether or not the given attestation contains a latest messages. - pub fn should_process_attestation(&self, state: &BeaconState, attestation: &Attestation) -> bool { + /// Determines whether or not the given attestation contains a latest message. + pub fn should_process_attestation(&self, state: &BeaconState, attestation: &Attestation) -> Result { 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 for Error { Error::BackendError(e) } } + diff --git a/beacon_node/rpc/src/attestation.rs b/beacon_node/rpc/src/attestation.rs index b85d4e9475..48d9eb4693 100644 --- a/beacon_node/rpc/src/attestation.rs +++ b/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 AttestationService for AttestationServiceInstance { resp.set_success(true); } - Err(e) => { + Err(BeaconChainError::AttestationValidationError(e)) => { // Attestation was invalid warn!( self.log, @@ -170,6 +170,28 @@ impl AttestationService for AttestationServiceInstance { 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();