From d53d43844c611b2d3fb0c1e916ba539a96168de0 Mon Sep 17 00:00:00 2001
From: Paul Hauner <paul@paulhauner.com>
Date: Tue, 21 Feb 2023 11:05:36 +1100
Subject: [PATCH] Suggestions for Capella `beacon_chain` (#3999)

* Remove CapellaReadiness::NotSynced

Some EEs have a habit of flipping between synced/not-synced, which causes some
spurious "Not read for the merge" messages back before the merge. For the
merge, if the EE wasn't synced the CE simple wouldn't go through the transition
(due to optimistic sync stuff). However, we don't have that hard requirement
for Capella; the CE will go through the fork and just wait for the EE to catch
up. I think that removing `NotSynced` here will avoid false-positives on the
"Not ready logs..". We'll be creating other WARN/ERRO logs if the EE isn't
synced, anyway.

* Change some Capella readiness logging

There's two changes here:

1. Shorten the log messages, for readability.
2. Change the hints.

Connecting a Capella-ready LH to a non-Capella-ready EE gives this log:

```
WARN Not ready for Capella                   info: The execution endpoint does not appear to support the required engine api methods for Capella: Required Methods Unsupported: engine_getPayloadV2 engine_forkchoiceUpdatedV2 engine_newPayloadV2, service: slot_notifier
```

This variant of error doesn't get a "try updating" style hint, when it's the
one that needs it. This is because we detect the method-not-found reponse from
the EE and return default capabilities, rather than indicating that the request
fails. I think it's fair to say that an EE upgrade is required whenever it
doesn't provide the required methods.

I changed the `ExchangeCapabilitiesFailed` message since that can only happen
when the EE fails to respond with anything other than success or not-found.
---
 .../beacon_chain/src/capella_readiness.rs       | 17 ++---------------
 beacon_node/client/src/notifier.rs              |  3 ++-
 2 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/beacon_node/beacon_chain/src/capella_readiness.rs b/beacon_node/beacon_chain/src/capella_readiness.rs
index b156321058..bb729d8999 100644
--- a/beacon_node/beacon_chain/src/capella_readiness.rs
+++ b/beacon_node/beacon_chain/src/capella_readiness.rs
@@ -21,8 +21,6 @@ pub const ENGINE_CAPABILITIES_REFRESH_INTERVAL: u64 = 300;
 pub enum CapellaReadiness {
     /// The execution engine is capella-enabled (as far as we can tell)
     Ready,
-    /// The EL can be reached and has the correct configuration, however it's not yet synced.
-    NotSynced,
     /// We are connected to an execution engine which doesn't support the V2 engine api methods
     V2MethodsNotSupported { error: String },
     /// The transition configuration with the EL failed, there might be a problem with
@@ -44,11 +42,6 @@ impl fmt::Display for CapellaReadiness {
                     execution endpoint: {}",
                 error
             ),
-            CapellaReadiness::NotSynced => write!(
-                f,
-                "The execution endpoint is connected and configured, \
-                    however it is not yet synced"
-            ),
             CapellaReadiness::NoExecutionEndpoint => write!(
                 f,
                 "The --execution-endpoint flag is not specified, this is a \
@@ -56,8 +49,7 @@ impl fmt::Display for CapellaReadiness {
             ),
             CapellaReadiness::V2MethodsNotSupported { error } => write!(
                 f,
-                "The execution endpoint does not appear to support \
-                    the required engine api methods for Capella: {}",
+                "Execution endpoint does not support Capella methods: {}",
                 error
             ),
         }
@@ -115,12 +107,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     }
 
                     if all_good {
-                        if !el.is_synced_for_notifier().await {
-                            // The EL is not synced.
-                            CapellaReadiness::NotSynced
-                        } else {
-                            CapellaReadiness::Ready
-                        }
+                        CapellaReadiness::Ready
                     } else {
                         CapellaReadiness::V2MethodsNotSupported {
                             error: missing_methods,
diff --git a/beacon_node/client/src/notifier.rs b/beacon_node/client/src/notifier.rs
index c1d830bc08..fb8a9b6349 100644
--- a/beacon_node/client/src/notifier.rs
+++ b/beacon_node/client/src/notifier.rs
@@ -466,13 +466,14 @@ async fn capella_readiness_logging<T: BeaconChainTypes>(
             error!(
                 log,
                 "Not ready for Capella";
+                "hint" => "the execution endpoint may be offline",
                 "info" => %readiness,
-                "hint" => "try updating Lighthouse and/or the execution layer",
             )
         }
         readiness => warn!(
             log,
             "Not ready for Capella";
+            "hint" => "try updating the execution endpoint",
             "info" => %readiness,
         ),
     }