From 5e3fb13cfe71c4c1aff6c82839a5b9a13db850ca Mon Sep 17 00:00:00 2001
From: Paul Hauner <paul@paulhauner.com>
Date: Wed, 7 Jun 2023 01:50:34 +0000
Subject: [PATCH] Downgrade a `CRIT` in the VC for builder timeouts (#4366)

## Issue Addressed

NA

## Proposed Changes

Downgrade a `CRIT` to an `ERRO` when there's an `Irrecoverable` error whilst publishing a blinded block.

It's quite common for builders successfully broadcast a block to the network whilst failing to respond to the BN when it publishes a signed, blinded block. The VC is currently raising a `CRIT` when this happens and I think that's excessive.

These changes have the same intent as  #4073. In that PR I only managed to remove the `CRIT`s in the BN but missed this one in the VC.

I've also tidied the log messages to:

- Give them all the same title (*"Error whilst producing block"*) to help with grepping.
- Include the `block_slot` so it's easy to look up the slot in an explorer and see if it was actually skipped.

## Additional Info

This PR should not change any logic beyond logging.
---
 validator_client/src/block_service.rs | 68 ++++++++++++++++++---------
 1 file changed, 47 insertions(+), 21 deletions(-)

diff --git a/validator_client/src/block_service.rs b/validator_client/src/block_service.rs
index 61a5a094c..d22e6c95f 100644
--- a/validator_client/src/block_service.rs
+++ b/validator_client/src/block_service.rs
@@ -338,35 +338,61 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockService<T, E> {
             let log = log.clone();
             self.inner.context.executor.spawn(
                 async move {
-                    let publish_result = if builder_proposals {
-                        let mut result = service.clone()
+                    if builder_proposals {
+                        let result = service
+                            .clone()
                             .publish_block::<BlindedPayload<E>>(slot, validator_pubkey)
                             .await;
-                        match result.as_ref() {
+                        match result {
                             Err(BlockError::Recoverable(e)) => {
-                                error!(log, "Error whilst producing a blinded block, attempting to \
-                                    publish full block"; "error" => ?e);
-                                result = service
+                                error!(
+                                    log,
+                                    "Error whilst producing block";
+                                    "error" => ?e,
+                                    "block_slot" => ?slot,
+                                    "info" => "blinded proposal failed, attempting full block"
+                                );
+                                if let Err(e) = service
                                     .publish_block::<FullPayload<E>>(slot, validator_pubkey)
-                                    .await;
-                            },
-                            Err(BlockError::Irrecoverable(e))  => {
-                                error!(log, "Error whilst producing a blinded block, cannot fallback \
-                                    because the block was signed"; "error" => ?e);
-                            },
-                            _ => {},
+                                    .await
+                                {
+                                    // Log a `crit` since a full block
+                                    // (non-builder) proposal failed.
+                                    crit!(
+                                        log,
+                                        "Error whilst producing block";
+                                        "error" => ?e,
+                                        "block_slot" => ?slot,
+                                        "info" => "full block attempted after a blinded failure",
+                                    );
+                                }
+                            }
+                            Err(BlockError::Irrecoverable(e)) => {
+                                // Only log an `error` since it's common for
+                                // builders to timeout on their response, only
+                                // to publish the block successfully themselves.
+                                error!(
+                                    log,
+                                    "Error whilst producing block";
+                                    "error" => ?e,
+                                    "block_slot" => ?slot,
+                                    "info" => "this error may or may not result in a missed block",
+                                )
+                            }
+                            Ok(_) => {}
                         };
-                        result
-                    } else {
-                        service
-                            .publish_block::<FullPayload<E>>(slot, validator_pubkey)
-                            .await
-                    };
-                    if let Err(e) = publish_result {
+                    } else if let Err(e) = service
+                        .publish_block::<FullPayload<E>>(slot, validator_pubkey)
+                        .await
+                    {
+                        // Log a `crit` since a full block (non-builder)
+                        // proposal failed.
                         crit!(
                             log,
                             "Error whilst producing block";
-                            "message" => ?e
+                            "message" => ?e,
+                            "block_slot" => ?slot,
+                            "info" => "proposal did not use a builder",
                         );
                     }
                 },