Return a result object instead of void when publishing execution payload (#10158)

Author: StefanBratanov

beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandler.java

@@ -115,6 +115,7 @@
 import tech.pegasys.teku.storage.client.CombinedChainDataClient;
 import tech.pegasys.teku.validator.api.CommitteeSubscriptionRequest;
 import tech.pegasys.teku.validator.api.NodeSyncingException;
+import tech.pegasys.teku.validator.api.PublishSignedExecutionPayloadResult;
 import tech.pegasys.teku.validator.api.SendSignedBlockResult;
 import tech.pegasys.teku.validator.api.SubmitDataError;
 import tech.pegasys.teku.validator.api.ValidatorApiChannel;
@@ -987,9 +988,16 @@ public SafeFuture<Optional<ExecutionPayloadEnvelope>> createUnsignedExecutionPay
   }
 
   @Override
-  public SafeFuture<Void> publishSignedExecutionPayload(
+  public SafeFuture<PublishSignedExecutionPayloadResult> publishSignedExecutionPayload(
       final SignedExecutionPayloadEnvelope signedExecutionPayload) {
-    return executionPayloadPublisher.publishSignedExecutionPayload(signedExecutionPayload);
+    return executionPayloadPublisher
+        .publishSignedExecutionPayload(signedExecutionPayload)
+        .exceptionally(
+            ex -> {
+              final String reason = getRootCauseMessage(ex);
+              return PublishSignedExecutionPayloadResult.rejected(
+                  signedExecutionPayload.getBeaconBlockRoot(), reason);
+            });
   }
 
   private Optional<SubmitDataError> fromInternalValidationResult(

beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/ExecutionPayloadPublisher.java

@@ -15,19 +15,17 @@
 
 import tech.pegasys.teku.infrastructure.async.SafeFuture;
 import tech.pegasys.teku.spec.datastructures.epbs.versions.gloas.SignedExecutionPayloadEnvelope;
+import tech.pegasys.teku.validator.api.PublishSignedExecutionPayloadResult;
 
 /** Used to publish execution payload and data column sidecars */
 public interface ExecutionPayloadPublisher {
 
   ExecutionPayloadPublisher NOOP =
-      new ExecutionPayloadPublisher() {
-        @Override
-        public SafeFuture<Void> publishSignedExecutionPayload(
-            final SignedExecutionPayloadEnvelope signedExecutionPayload) {
-          return SafeFuture.COMPLETE;
-        }
-      };
+      signedExecutionPayload ->
+          SafeFuture.completedFuture(
+              PublishSignedExecutionPayloadResult.success(
+                  signedExecutionPayload.getBeaconBlockRoot()));
 
-  SafeFuture<Void> publishSignedExecutionPayload(
+  SafeFuture<PublishSignedExecutionPayloadResult> publishSignedExecutionPayload(
       SignedExecutionPayloadEnvelope signedExecutionPayload);
 }

beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/ExecutionPayloadPublisherGloas.java

@@ -16,13 +16,15 @@
 import java.util.List;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.tuweni.bytes.Bytes32;
 import tech.pegasys.teku.infrastructure.async.SafeFuture;
 import tech.pegasys.teku.networking.eth2.gossip.DataColumnSidecarGossipChannel;
 import tech.pegasys.teku.networking.eth2.gossip.ExecutionPayloadGossipChannel;
 import tech.pegasys.teku.spec.datastructures.blobs.DataColumnSidecar;
 import tech.pegasys.teku.spec.datastructures.epbs.versions.gloas.SignedExecutionPayloadEnvelope;
 import tech.pegasys.teku.statetransition.blobs.RemoteOrigin;
 import tech.pegasys.teku.statetransition.execution.ExecutionPayloadManager;
+import tech.pegasys.teku.validator.api.PublishSignedExecutionPayloadResult;
 import tech.pegasys.teku.validator.coordinator.ExecutionPayloadFactory;
 
 public class ExecutionPayloadPublisherGloas implements ExecutionPayloadPublisher {
@@ -46,13 +48,26 @@ public ExecutionPayloadPublisherGloas(
   }
 
   @Override
-  public SafeFuture<Void> publishSignedExecutionPayload(
+  public SafeFuture<PublishSignedExecutionPayloadResult> publishSignedExecutionPayload(
       final SignedExecutionPayloadEnvelope signedExecutionPayload) {
-    // we publish the execution payload (and data column sidecars) immediately and then import
-    publishExecutionPayloadAndDataColumnSidecars(
-        signedExecutionPayload,
-        executionPayloadFactory.createDataColumnSidecars(signedExecutionPayload));
-    return executionPayloadManager.importExecutionPayload(signedExecutionPayload).toVoid();
+    return executionPayloadManager
+        .validateAndImportExecutionPayload(signedExecutionPayload)
+        .thenApply(
+            result -> {
+              final Bytes32 beaconBlockRoot = signedExecutionPayload.getBeaconBlockRoot();
+              if (result.isAccept()) {
+                // we publish the execution payload (and data column sidecars) after passing gossip
+                // validation
+                publishExecutionPayloadAndDataColumnSidecars(
+                    signedExecutionPayload,
+                    executionPayloadFactory.createDataColumnSidecars(signedExecutionPayload));
+                return PublishSignedExecutionPayloadResult.success(beaconBlockRoot);
+              }
+              return PublishSignedExecutionPayloadResult.rejected(
+                  beaconBlockRoot,
+                  "Failed gossip validation"
+                      + result.getDescription().map(description -> ": " + description).orElse(""));
+            });
   }
 
   private void publishExecutionPayloadAndDataColumnSidecars(

beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java

@@ -123,6 +123,7 @@
 import tech.pegasys.teku.storage.client.CombinedChainDataClient;
 import tech.pegasys.teku.validator.api.CommitteeSubscriptionRequest;
 import tech.pegasys.teku.validator.api.NodeSyncingException;
+import tech.pegasys.teku.validator.api.PublishSignedExecutionPayloadResult;
 import tech.pegasys.teku.validator.api.SendSignedBlockResult;
 import tech.pegasys.teku.validator.api.SubmitDataError;
 import tech.pegasys.teku.validator.coordinator.performance.DefaultPerformanceTracker;
@@ -1340,12 +1341,29 @@ public void createUnsignedExecutionPayload_shouldCreateExecutionPayload() {
   public void publishSignedExecutionPayload_shouldPublish() {
     final SignedExecutionPayloadEnvelope signedExecutionPayload =
         dataStructureUtil.randomSignedExecutionPayloadEnvelope(5);
+    final PublishSignedExecutionPayloadResult publishResult =
+        PublishSignedExecutionPayloadResult.success(signedExecutionPayload.getBeaconBlockRoot());
     when(executionPayloadPublisher.publishSignedExecutionPayload(eq(signedExecutionPayload)))
-        .thenReturn(SafeFuture.COMPLETE);
-    final SafeFuture<Void> result =
-        validatorApiHandler.publishSignedExecutionPayload(signedExecutionPayload);
+        .thenReturn(SafeFuture.completedFuture(publishResult));
 
-    assertThat(result).isCompleted();
+    assertThat(validatorApiHandler.publishSignedExecutionPayload(signedExecutionPayload))
+        .isCompletedWithValue(publishResult);
+
+    verify(executionPayloadPublisher).publishSignedExecutionPayload(signedExecutionPayload);
+  }
+
+  @Test
+  public void publishSignedExecutionPayload_shouldHandleExceptions() {
+    final SignedExecutionPayloadEnvelope signedExecutionPayload =
+        dataStructureUtil.randomSignedExecutionPayloadEnvelope(5);
+    final PublishSignedExecutionPayloadResult failedResult =
+        PublishSignedExecutionPayloadResult.rejected(
+            signedExecutionPayload.getBeaconBlockRoot(), "oopsy");
+    when(executionPayloadPublisher.publishSignedExecutionPayload(eq(signedExecutionPayload)))
+        .thenReturn(SafeFuture.failedFuture(new IllegalStateException("oopsy")));
+
+    assertThat(validatorApiHandler.publishSignedExecutionPayload(signedExecutionPayload))
+        .isCompletedWithValue(failedResult);
 
     verify(executionPayloadPublisher).publishSignedExecutionPayload(signedExecutionPayload);
   }

beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/ExecutionPayloadPublisherGloasTest.java

@@ -13,14 +13,14 @@
 
 package tech.pegasys.teku.validator.coordinator.publisher;
 
-import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
 
 import java.util.List;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.mockito.InOrder;
 import tech.pegasys.teku.infrastructure.async.SafeFuture;
 import tech.pegasys.teku.infrastructure.async.SafeFutureAssert;
 import tech.pegasys.teku.networking.eth2.gossip.DataColumnSidecarGossipChannel;
@@ -29,10 +29,11 @@
 import tech.pegasys.teku.spec.TestSpecFactory;
 import tech.pegasys.teku.spec.datastructures.blobs.DataColumnSidecar;
 import tech.pegasys.teku.spec.datastructures.epbs.versions.gloas.SignedExecutionPayloadEnvelope;
-import tech.pegasys.teku.spec.logic.common.statetransition.results.ExecutionPayloadImportResult;
 import tech.pegasys.teku.spec.util.DataStructureUtil;
 import tech.pegasys.teku.statetransition.blobs.RemoteOrigin;
 import tech.pegasys.teku.statetransition.execution.ExecutionPayloadManager;
+import tech.pegasys.teku.statetransition.validation.InternalValidationResult;
+import tech.pegasys.teku.validator.api.PublishSignedExecutionPayloadResult;
 import tech.pegasys.teku.validator.coordinator.ExecutionPayloadFactory;
 
 class ExecutionPayloadPublisherGloasTest {
@@ -64,30 +65,37 @@ class ExecutionPayloadPublisherGloasTest {
 
   @BeforeEach
   public void setUp() {
+    when(executionPayloadManager.validateAndImportExecutionPayload(signedExecutionPayload))
+        .thenReturn(SafeFuture.completedFuture(InternalValidationResult.ACCEPT));
     when(executionPayloadFactory.createDataColumnSidecars(signedExecutionPayload))
         .thenReturn(SafeFuture.completedFuture(dataColumnSidecars));
     when(executionPayloadGossipChannel.publishExecutionPayload(signedExecutionPayload))
         .thenReturn(SafeFuture.COMPLETE);
-    when(executionPayloadManager.importExecutionPayload(signedExecutionPayload))
-        .thenReturn(
-            SafeFuture.completedFuture(
-                ExecutionPayloadImportResult.successful(signedExecutionPayload)));
   }
 
   @Test
-  public void publishSignedExecutionPayload_shouldPublishImmediatelyAndImport() {
+  public void publishSignedExecutionPayload_shouldValidateAndPublish() {
     SafeFutureAssert.assertThatSafeFuture(
             executionPayloadPublisher.publishSignedExecutionPayload(signedExecutionPayload))
-        .isCompleted();
+        .isCompletedWithValue(
+            PublishSignedExecutionPayloadResult.success(
+                signedExecutionPayload.getBeaconBlockRoot()));
 
-    final InOrder inOrder =
-        inOrder(
-            executionPayloadGossipChannel, dataColumnSidecarGossipChannel, executionPayloadManager);
-
-    inOrder.verify(executionPayloadGossipChannel).publishExecutionPayload(signedExecutionPayload);
-    inOrder
-        .verify(dataColumnSidecarGossipChannel)
+    verify(executionPayloadGossipChannel).publishExecutionPayload(signedExecutionPayload);
+    verify(dataColumnSidecarGossipChannel)
         .publishDataColumnSidecars(dataColumnSidecars, RemoteOrigin.LOCAL_PROPOSAL);
-    inOrder.verify(executionPayloadManager).importExecutionPayload(signedExecutionPayload);
+  }
+
+  @Test
+  public void publishSignedExecutionPayload_shouldReturnRejectedResultIfGossipValidationFails() {
+    when(executionPayloadManager.validateAndImportExecutionPayload(signedExecutionPayload))
+        .thenReturn(SafeFuture.completedFuture(InternalValidationResult.reject("oopsy")));
+    SafeFutureAssert.assertThatSafeFuture(
+            executionPayloadPublisher.publishSignedExecutionPayload(signedExecutionPayload))
+        .isCompletedWithValue(
+            PublishSignedExecutionPayloadResult.rejected(
+                signedExecutionPayload.getBeaconBlockRoot(), "Failed gossip validation: oopsy"));
+
+    verifyNoInteractions(executionPayloadGossipChannel, dataColumnSidecarGossipChannel);
   }
 }

ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/epbs/versions/gloas/SignedExecutionPayloadEnvelope.java

@@ -13,6 +13,7 @@
 
 package tech.pegasys.teku.spec.datastructures.epbs.versions.gloas;
 
+import org.apache.tuweni.bytes.Bytes32;
 import tech.pegasys.teku.bls.BLSSignature;
 import tech.pegasys.teku.infrastructure.logging.LogFormatter;
 import tech.pegasys.teku.infrastructure.ssz.containers.Container2;
@@ -53,6 +54,10 @@ public UInt64 getSlot() {
     return getMessage().getSlot();
   }
 
+  public Bytes32 getBeaconBlockRoot() {
+    return getMessage().getBeaconBlockRoot();
+  }
+
   public SlotAndBlockRoot getSlotAndBlockRoot() {
     return getMessage().getSlotAndBlockRoot();
   }

ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/forkchoice/MutableStore.java

@@ -22,6 +22,7 @@
 import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock;
 import tech.pegasys.teku.spec.datastructures.blocks.SignedBlockAndState;
 import tech.pegasys.teku.spec.datastructures.blocks.SlotAndBlockRoot;
+import tech.pegasys.teku.spec.datastructures.epbs.versions.gloas.SignedExecutionPayloadEnvelope;
 import tech.pegasys.teku.spec.datastructures.state.Checkpoint;
 import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState;
 
@@ -67,6 +68,15 @@ default void putBlockAndState(
         Optional.empty());
   }
 
+  /**
+   * Stores the corresponding data for this execution payload
+   *
+   * @param executionPayload Execution payload
+   * @param state Corresponding state
+   */
+  void putExecutionPayloadAndState(
+      SignedExecutionPayloadEnvelope executionPayload, BeaconState state);
+
   void putStateRoot(Bytes32 stateRoot, SlotAndBlockRoot slotAndBlockRoot);
 
   void pullUpBlockCheckpoints(Bytes32 blockRoot);

ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/execution/AbstractExecutionPayloadProcessor.java

@@ -47,7 +47,7 @@ public BeaconState processAndVerifyExecutionPayload(
               "State transition error while importing execution payload (builder index: %s, slot: %s, block root: %s)",
               signedEnvelope.getMessage().getBuilderIndex(),
               signedEnvelope.getMessage().getSlot(),
-              signedEnvelope.getMessage().getBeaconBlockRoot()),
+              signedEnvelope.getBeaconBlockRoot()),
           ex);
       throw new StateTransitionException(ex);
     }

ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/util/ForkChoiceUtil.java

@@ -548,4 +548,9 @@ private boolean isExecutionBlock(final ReadOnlyStore store, final SignedBeaconBl
   public AvailabilityChecker<?> createAvailabilityChecker(final SignedBeaconBlock block) {
     return AvailabilityChecker.NOOP;
   }
+
+  public AvailabilityChecker<?> createAvailabilityChecker(
+      final SignedExecutionPayloadEnvelope executionPayload) {
+    return AvailabilityChecker.NOOP;
+  }
 }

ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/gloas/util/ForkChoiceUtilGloas.java

@@ -40,15 +40,21 @@ public void applyExecutionPayloadToStore(
       final MutableStore store,
       final SignedExecutionPayloadEnvelope signedEnvelope,
       final BeaconState postState) {
-    // TODO-GLOAS: https://github.com/Consensys/teku/issues/9878
+    // Add new execution payload to store
+    store.putExecutionPayloadAndState(signedEnvelope, postState);
   }
 
+  // Checking of blob data availability is delayed until the processing of the execution payload
   @Override
   public AvailabilityChecker<?> createAvailabilityChecker(final SignedBeaconBlock block) {
-    // TODO-GLOAS: in ePBS, data availability is delayed until the processing of the execution
-    // payload.
-    // We may have a dedicated availability checker for the execution stage.
-    // If it will be the case, this will remain a NOOP
-    return AvailabilityChecker.NOOP;
+    return AvailabilityChecker.NOOP_DATACOLUMN_SIDECAR;
+  }
+
+  // TODO-GLOAS: https://github.com/Consensys/teku/issues/9878 add a real data availability check
+  // (not required for devnet-0)
+  @Override
+  public AvailabilityChecker<?> createAvailabilityChecker(
+      final SignedExecutionPayloadEnvelope executionPayload) {
+    return AvailabilityChecker.NOOP_DATACOLUMN_SIDECAR;
   }
 }