From 1c3c3bd1b0b3fb8cd21b338e05a66d113ce83345 Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Tue, 10 May 2022 13:21:31 +0300 Subject: [PATCH] do not use BuilderStatus object --- .../ExecutionLayerManagerImpl.java | 53 ++++++--------- .../ExecutionLayerManagerImplTest.java | 4 ++ .../execution/BuilderStatus.java | 67 ------------------- 3 files changed, 23 insertions(+), 101 deletions(-) delete mode 100644 ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/BuilderStatus.java diff --git a/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImpl.java b/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImpl.java index 380365ebb9a..f778cc0eea5 100644 --- a/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImpl.java +++ b/ethereum/executionlayer/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; + private final AtomicBoolean latestBuilderAvailability; + private final Spec spec; private final EventLogger eventLogger; - private final AtomicReference latestBuilderStatus = new AtomicReference<>(); - public static ExecutionLayerManagerImpl create( final Web3JClient engineWeb3JClient, final Optional builderWeb3JClient, @@ -116,13 +115,14 @@ private static Optional 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 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 unwrapResponseOrThrow(Response response) { return checkNotNull(response.getPayload(), "No payload content found"); } - private SafeFuture 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/ethereum/executionlayer/src/test/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImplTest.java b/ethereum/executionlayer/src/test/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImplTest.java index 876a00bc941..8ddd01d752a 100644 --- a/ethereum/executionlayer/src/test/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImplTest.java +++ b/ethereum/executionlayer/src/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/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/BuilderStatus.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/BuilderStatus.java deleted file mode 100644 index 09666200276..00000000000 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/BuilderStatus.java +++ /dev/null @@ -1,67 +0,0 @@ -/* - * Copyright 2022 ConsenSys AG. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package tech.pegasys.teku.spec.datastructures.execution; - -import com.google.common.base.MoreObjects; - -public class BuilderStatus { - - private final Status status; - private final String errorMessage; - - private BuilderStatus(final Status status, final String errorMessage) { - this.status = status; - this.errorMessage = errorMessage; - } - - public static BuilderStatus withOkStatus() { - return new BuilderStatus(Status.OK, null); - } - - public static BuilderStatus withFailedStatus(final String errorMessage) { - return new BuilderStatus(null, errorMessage); - } - - public static BuilderStatus withFailedStatus(final Throwable throwable) { - return withFailedStatus(throwable.getMessage()); - } - - public Status getStatus() { - return status; - } - - public String getErrorMessage() { - return errorMessage; - } - - public boolean isOk() { - return status == Status.OK && !isFailed(); - } - - public boolean isFailed() { - return errorMessage != null; - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("status", status) - .add("errorMessage", errorMessage) - .toString(); - } - - public enum Status { - OK - } -}