do not use BuilderStatus object

Consensys · May 10, 2022 · 1c3c3bd · 1c3c3bd
1 parent 3f7ca3b
commit 1c3c3bd
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 101 deletions.
diff --git a/...er/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImpl.java b/...er/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImpl.java
@@ -21,7 +21,7 @@
 import static tech.pegasys.teku.spec.config.Constants.MAXIMUM_CONCURRENT_EE_REQUESTS;
 
 import java.util.Optional;
-import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.atomic.AtomicBoolean;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.tuweni.bytes.Bytes32;
@@ -50,7 +50,6 @@
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
 import tech.pegasys.teku.spec.Spec;
 import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock;
-import tech.pegasys.teku.spec.datastructures.execution.BuilderStatus;
 import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayload;
 import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadContext;
 import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadHeader;
@@ -68,12 +67,12 @@ public class ExecutionLayerManagerImpl implements ExecutionLayerManager {
   private final ExecutionEngineClient executionEngineClient;
   private final Optional<ExecutionBuilderClient> executionBuilderClient;
 
+  private final AtomicBoolean latestBuilderAvailability;
+
   private final Spec spec;
 
   private final EventLogger eventLogger;
 
-  private final AtomicReference<BuilderStatus> latestBuilderStatus = new AtomicReference<>();
-
   public static ExecutionLayerManagerImpl create(
       final Web3JClient engineWeb3JClient,
       final Optional<Web3JClient> builderWeb3JClient,
@@ -116,13 +115,14 @@ private static Optional<ExecutionBuilderClient> createBuilderClient(
       final EventLogger eventLogger) {
     this.executionEngineClient = executionEngineClient;
     this.executionBuilderClient = executionBuilderClient;
+    this.latestBuilderAvailability = new AtomicBoolean(executionBuilderClient.isPresent());
     this.spec = spec;
     this.eventLogger = eventLogger;
   }
 
   @Override
   public void onSlot(UInt64 slot) {
-    updateBuilderStatus();
+    updateBuilderAvailability();
   }
 
   @Override
@@ -232,9 +232,7 @@ public SafeFuture<TransitionConfiguration> engineExchangeTransitionConfiguration
   }
 
   boolean isBuilderAvailable() {
-    return Optional.ofNullable(latestBuilderStatus.get())
-        .map(BuilderStatus::isOk)
-        .orElse(executionBuilderClient.isPresent());
+    return latestBuilderAvailability.get();
   }
 
   @Override
@@ -319,40 +317,27 @@ private static <K> K unwrapResponseOrThrow(Response<K> response) {
     return checkNotNull(response.getPayload(), "No payload content found");
   }
 
-  private SafeFuture<BuilderStatus> retrieveBuilderStatus() {
+  private void updateBuilderAvailability() {
     if (executionBuilderClient.isEmpty()) {
-      return SafeFuture.completedFuture(null);
+      return;
     }
-    return executionBuilderClient
+    executionBuilderClient
         .get()
         .status()
-        .thenApply(
+        .finish(
             statusResponse -> {
-              if (statusResponse.getErrorMessage() == null) {
-                return BuilderStatus.withOkStatus();
-              } else {
-                return BuilderStatus.withFailedStatus(statusResponse.getErrorMessage());
-              }
-            })
-        .exceptionally(BuilderStatus::withFailedStatus);
-  }
-
-  private void updateBuilderStatus() {
-    if (executionBuilderClient.isEmpty()) {
-      return;
-    }
-    retrieveBuilderStatus()
-        .thenAccept(
-            status -> {
-              BuilderStatus previousStatus = latestBuilderStatus.getAndSet(status);
-              if (status.isFailed()) {
-                eventLogger.executionBuilderIsOffline(status.getErrorMessage());
+              if (statusResponse.getErrorMessage() != null) {
+                latestBuilderAvailability.set(false);
+                eventLogger.executionBuilderIsOffline(statusResponse.getErrorMessage());
               } else {
-                if (previousStatus != null && previousStatus.isFailed()) {
+                if (latestBuilderAvailability.compareAndSet(false, true)) {
                   eventLogger.executionBuilderIsBackOnline();
                 }
               }
-            })
-        .reportExceptions();
+            },
+            throwable -> {
+              latestBuilderAvailability.set(false);
+              eventLogger.executionBuilderIsOffline(throwable.getMessage());
+            });
   }
 }
diff --git a/...rc/test/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImplTest.java b/...rc/test/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImplTest.java
@@ -54,6 +54,7 @@ public void builderShouldNotBeAvailableWhenBuilderNotEnabled() {
     noBuilderEnabled.onSlot(UInt64.ONE);
 
     assertThat(noBuilderEnabled.isBuilderAvailable()).isFalse();
+    verifyNoInteractions(executionBuilderClient);
   }
 
   @Test
@@ -88,6 +89,9 @@ public void builderShouldNotBeAvailableWhenBuilderStatusCallFails() {
 
   @Test
   public void builderAvailabilityIsUpdatedOnSlotEventAndLoggedAdequately() {
+    // Initially builder should be available
+    assertThat(underTest.isBuilderAvailable()).isTrue();
+
     // Given builder status is ok
     updateBuilderStatus(SafeFuture.completedFuture(new Response<>(GenericBuilderStatus.OK)));
 

diff --git a/...eum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/BuilderStatus.java b/...eum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/BuilderStatus.java