From 51ffbc07d25644ef7a8ef0cea64a5d316ec8ec18 Mon Sep 17 00:00:00 2001
From: Age Manning <Age@AgeManning.com>
Date: Sat, 30 Mar 2019 19:48:45 +1100
Subject: [PATCH] Correct attestation error handling

---
 beacon_node/rpc/src/attestation.rs            |  5 +++--
 .../src/attestation_producer/mod.rs           | 21 +++++++++++++++----
 validator_client/src/block_producer/mod.rs    |  4 ++++
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/beacon_node/rpc/src/attestation.rs b/beacon_node/rpc/src/attestation.rs
index f9423df61..aec948abb 100644
--- a/beacon_node/rpc/src/attestation.rs
+++ b/beacon_node/rpc/src/attestation.rs
@@ -25,7 +25,7 @@ impl AttestationService for AttestationServiceInstance {
         req: ProduceAttestationDataRequest,
         sink: UnarySink<ProduceAttestationDataResponse>,
     ) {
-        warn!(
+        trace!(
             &self.log,
             "Attempting to produce attestation at slot {}",
             req.get_slot()
@@ -92,7 +92,7 @@ impl AttestationService for AttestationServiceInstance {
         req: PublishAttestationRequest,
         sink: UnarySink<PublishAttestationResponse>,
     ) {
-        warn!(self.log, "Publishing attestation");
+        trace!(self.log, "Publishing attestation");
 
         let mut resp = PublishAttestationResponse::new();
         let ssz_serialized_attestation = req.get_attestation().get_ssz();
@@ -128,6 +128,7 @@ impl AttestationService for AttestationServiceInstance {
                     self.log,
                     "PublishAttestation";
                     "type" => "invalid_attestation",
+                    "error" => format!("{:?}", e),
                 );
                 resp.set_success(false);
                 resp.set_msg(format!("InvalidAttestation: {:?}", e).as_bytes().to_vec());
diff --git a/validator_client/src/attestation_producer/mod.rs b/validator_client/src/attestation_producer/mod.rs
index 7b2174b0c..db9028c40 100644
--- a/validator_client/src/attestation_producer/mod.rs
+++ b/validator_client/src/attestation_producer/mod.rs
@@ -4,7 +4,7 @@ mod grpc;
 use std::sync::Arc;
 use types::{BeaconBlock, ChainSpec, Domain, Fork, Slot};
 //TODO: Move these higher up in the crate
-use super::block_producer::{BeaconNodeError, ValidatorEvent};
+use super::block_producer::{BeaconNodeError, PublishOutcome, ValidatorEvent};
 use crate::signer::Signer;
 use beacon_node_attestation::BeaconNodeAttestation;
 use slog::{error, info, warn};
@@ -58,6 +58,12 @@ impl<'a, B: BeaconNodeAttestation, S: Signer> AttestationProducer<'a, B, S> {
             Ok(ValidatorEvent::BeaconNodeUnableToProduceAttestation(_slot)) => {
                 error!(log, "Attestation production error"; "Error" => format!("Beacon node was unable to produce an attestation"))
             }
+            Ok(ValidatorEvent::PublishAttestationFailed) => {
+                error!(log, "Attestation production error"; "Error" => format!("Beacon node was unable to publish an attestation"))
+            }
+            Ok(ValidatorEvent::InvalidAttestation) => {
+                error!(log, "Attestation production error"; "Error" => format!("The signed attestation was invalid"))
+            }
             Ok(v) => {
                 warn!(log, "Unknown result for attestation production"; "Error" => format!("{:?}",v))
             }
@@ -83,8 +89,15 @@ impl<'a, B: BeaconNodeAttestation, S: Signer> AttestationProducer<'a, B, S> {
         if self.safe_to_produce(&attestation) {
             let domain = self.spec.get_domain(epoch, Domain::Attestation, &self.fork);
             if let Some(attestation) = self.sign_attestation(attestation, self.duty, domain) {
-                self.beacon_node.publish_attestation(attestation)?;
-                Ok(ValidatorEvent::AttestationProduced(self.duty.slot))
+                match self.beacon_node.publish_attestation(attestation) {
+                    Ok(PublishOutcome::InvalidAttestation(_string)) => {
+                        Ok(ValidatorEvent::InvalidAttestation)
+                    }
+                    Ok(PublishOutcome::Valid) => {
+                        Ok(ValidatorEvent::AttestationProduced(self.duty.slot))
+                    }
+                    Err(_) | Ok(_) => Ok(ValidatorEvent::PublishAttestationFailed),
+                }
             } else {
                 Ok(ValidatorEvent::SignerRejection(self.duty.slot))
             }
@@ -101,7 +114,7 @@ impl<'a, B: BeaconNodeAttestation, S: Signer> AttestationProducer<'a, B, S> {
     /// done upstream.
     fn sign_attestation(
         &mut self,
-        mut attestation: AttestationData,
+        attestation: AttestationData,
         duties: AttestationDuty,
         domain: u64,
     ) -> Option<Attestation> {
diff --git a/validator_client/src/block_producer/mod.rs b/validator_client/src/block_producer/mod.rs
index 592c0b919..dc7f6c3ee 100644
--- a/validator_client/src/block_producer/mod.rs
+++ b/validator_client/src/block_producer/mod.rs
@@ -31,6 +31,10 @@ pub enum ValidatorEvent {
     BeaconNodeUnableToProduceAttestation(Slot),
     /// The signer failed to sign the message.
     SignerRejection(Slot),
+    /// Publishing an attestation failed.
+    PublishAttestationFailed,
+    /// Beacon node rejected the attestation.
+    InvalidAttestation,
 }
 
 /// This struct contains the logic for requesting and signing beacon blocks for a validator. The