return just one error in Error::EngineErrors

beacon_node/execution_layer/src/lib.rs

@@ -68,7 +68,7 @@ pub enum Error {
     NoPayloadBuilder,
     ApiError(ApiError),
     // TODO: remove Vec
-    EngineErrors(Vec<EngineError>),
+    EngineErrors(EngineError),
     NotSynced,
     ShuttingDown,
     FeeRecipientUnspecified,
@@ -631,8 +631,7 @@ impl ExecutionLayer {
                             .map_err(|_| ApiError::PayloadConversionLogicFlaw)
                     })
                     .await
-                    // TODO: fix here
-                    .map_err(|e| Error::EngineErrors(vec![e]))
+                    .map_err(Error::EngineErrors)
             }
             BlockType::Full => {
                 debug!(
@@ -713,8 +712,7 @@ impl ExecutionLayer {
                             .map(Into::into)
                     })
                     .await
-                    // TODO: fix here
-                    .map_err(|e| Error::EngineErrors(vec![e]))
+                    .map_err(Error::EngineErrors)
             }
         }
     }
@@ -939,7 +937,6 @@ impl ExecutionLayer {
             .broadcast(|engine| engine.api.exchange_transition_configuration_v1(local))
             .await;
 
-        let mut errors = vec![];
         match broadcast_result {
             Ok(remote) => {
                 if local.terminal_total_difficulty != remote.terminal_total_difficulty
@@ -953,16 +950,17 @@ impl ExecutionLayer {
                         "remote" => ?remote,
                         "local" => ?local,
                     );
-                    errors.push(EngineError::Api {
+                    Err(Error::EngineErrors(EngineError::Api {
                         // TODO: fix here
                         id: 0.to_string(),
                         error: ApiError::TransitionConfigurationMismatch,
-                    });
+                    }))
                 } else {
                     debug!(
                         self.log(),
                         "Execution client config is OK";
                     );
+                    Ok(())
                 }
             }
             Err(e) => {
@@ -971,15 +969,9 @@ impl ExecutionLayer {
                     "Unable to get transition config";
                     "error" => ?e,
                 );
-                errors.push(e);
+                Err(Error::EngineErrors(e))
             }
         }
-
-        if errors.is_empty() {
-            Ok(())
-        } else {
-            Err(Error::EngineErrors(errors))
-        }
     }
 
     /// Used during block production to determine if the merge has been triggered.
@@ -1018,8 +1010,7 @@ impl ExecutionLayer {
                     .await
             })
             .await
-            // TODO: fix here
-            .map_err(|e| Error::EngineErrors(vec![e]))?;
+            .map_err(Error::EngineErrors)?;
 
         if let Some(hash) = &hash_opt {
             info!(
@@ -1116,8 +1107,7 @@ impl ExecutionLayer {
             &[metrics::IS_VALID_TERMINAL_POW_BLOCK_HASH],
         );
 
-        let broadcast_result = self
-            .engines()
+        self.engines()
             .broadcast(|engine| async move {
                 if let Some(pow_block) = self.get_pow_block(engine, block_hash).await? {
                     if let Some(pow_parent) =
@@ -1130,36 +1120,8 @@ impl ExecutionLayer {
                 }
                 Ok(None)
             })
-            .await;
-
-        let mut errors = vec![];
-        let mut terminal = 0;
-        let mut not_terminal = 0;
-        let mut block_missing = 0;
-        match broadcast_result {
-            Ok(Some(true)) => terminal += 1,
-            Ok(Some(false)) => not_terminal += 1,
-            Ok(None) => block_missing += 1,
-            Err(e) => errors.push(e),
-        }
-
-        if terminal > 0 && not_terminal > 0 {
-            crit!(
-                self.log(),
-                "Consensus failure between execution nodes";
-                "method" => "is_valid_terminal_pow_block_hash"
-            );
-        }
-
-        if terminal > 0 {
-            Ok(Some(true))
-        } else if not_terminal > 0 {
-            Ok(Some(false))
-        } else if block_missing > 0 {
-            Ok(None)
-        } else {
-            Err(Error::EngineErrors(errors))
-        }
+            .await
+            .map_err(Error::EngineErrors)
     }
 
     /// This function should remain internal.
@@ -1217,8 +1179,7 @@ impl ExecutionLayer {
                     .await
             })
             .await
-            // TODO: fix this
-            .map_err(|e| Error::EngineErrors(vec![e]))
+            .map_err(Error::EngineErrors)
     }
 
     async fn get_payload_by_block_hash_from_engine<T: EthSpec>(
@@ -1280,8 +1241,7 @@ impl ExecutionLayer {
                 engine.api.propose_blinded_block_v1(block.clone()).await
             })
             .await
-            // TODO: fix here
-            .map_err(|e| Error::EngineErrors(vec![e]))
+            .map_err(Error::EngineErrors)
     }
 }
 

beacon_node/execution_layer/src/payload_status.rs

@@ -48,6 +48,7 @@ pub fn process_multiple_payload_statuses(
     statuses: impl Iterator<Item = Result<PayloadStatusV1, EngineError>>,
     log: &Logger,
 ) -> Result<PayloadStatus, Error> {
+    // TODO: review the whole thing
     let mut errors = vec![];
     let mut valid_statuses = vec![];
     let mut invalid_statuses = vec![];
@@ -187,5 +188,13 @@ pub fn process_multiple_payload_statuses(
         .or_else(|| other_statuses.first())
         .cloned()
         .map(Result::Ok)
-        .unwrap_or_else(|| Err(Error::EngineErrors(errors)))
+        // TODO: fix this
+        .unwrap_or_else(|| {
+            Err(Error::EngineErrors(
+                errors
+                    .into_iter()
+                    .next()
+                    .expect("Surely we get here only if there are errors"),
+            ))
+        })
 }