use header based selector with slot fallback (#8522)

* use header based selector with slot fallback
Consensys · Aug 22, 2024 · 35da85a · 35da85a
1 parent 933df84
commit 35da85a
Show file tree

Hide file tree

Showing 9 changed files with 97 additions and 84 deletions.
diff --git a/...t/java/tech/pegasys/teku/beaconrestapi/v1/validator/PostBlindedAndUnblindedBlockTest.java b/...t/java/tech/pegasys/teku/beaconrestapi/v1/validator/PostBlindedAndUnblindedBlockTest.java
@@ -17,6 +17,7 @@
 import static org.mockito.Mockito.when;
 import static tech.pegasys.teku.beaconrestapi.v1.validator.PostBlindedAndUnblindedBlockTest.Version.V1;
 import static tech.pegasys.teku.beaconrestapi.v1.validator.PostBlindedAndUnblindedBlockTest.Version.V2;
+import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_BAD_REQUEST;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_OK;
 
 import java.io.IOException;
@@ -58,37 +59,22 @@ public static Stream<Arguments> postBlockCases() {
         .flatMap(
             route ->
                 Stream.of(
-                    // route, useSsz, versionHeader
-                    Arguments.of(route, false, Optional.empty()),
-                    // Methods using Eth-Consensus-Version header (only for SSZ, we could add it for
-                    // json too when we start using it)
-                    Arguments.of(route, true, Optional.empty()),
-
-                    // deneb header on bellatrix will fall back to ssz slot based selector, because
-                    // deneb is not configured.
-                    // It is irrelevant for json, because we currently always use the slot based
-                    // selector.
-                    Arguments.of(route, true, Optional.of("deneb"))))
+                    // route, useSsz
+                    Arguments.of(route, false), Arguments.of(route, true)))
         .map(
             args -> {
               final String route = (String) args.get()[0];
               final boolean isBlindedBlock = route.contains("blinded");
               final Version version = route.contains("/v2/") ? V2 : V1;
               final boolean useSsz = (boolean) args.get()[1];
-              @SuppressWarnings("unchecked")
-              final Optional<String> versionHeader = (Optional<String>) args.get()[2];
-              return Arguments.of(version, isBlindedBlock, route, useSsz, versionHeader);
+              return Arguments.of(version, isBlindedBlock, route, useSsz);
             });
   }
 
-  @ParameterizedTest(name = "version:{0}_blinded:{1}_ssz:{3}_versionHeader:{4}")
+  @ParameterizedTest(name = "version:{0}_blinded:{1}_ssz:{3}")
   @MethodSource("postBlockCases")
-  void shouldReturnOk(
-      final Version version,
-      final boolean isBlindedBlock,
-      final String route,
-      final boolean useSsz,
-      final Optional<String> versionHeader)
+  void preDeneb(
+      final Version version, final boolean isBlindedBlock, final String route, final boolean useSsz)
       throws IOException {
 
     startRestAPIAtGenesis(SpecMilestone.BELLATRIX);
@@ -111,17 +97,21 @@ void shouldReturnOk(
 
     prepareResponse(request, version);
 
-    postRequestAndAssert(route, request, signedBeaconBlockSchema, versionHeader, useSsz, version);
+    postRequestAndAssert(
+        route,
+        request,
+        signedBeaconBlockSchema,
+        Optional.of(SpecMilestone.BELLATRIX.name()),
+        useSsz,
+        version);
+    postRequestAndAssert(
+        route, request, signedBeaconBlockSchema, Optional.empty(), useSsz, version);
   }
 
-  @ParameterizedTest(name = "version:{0}_blinded:{1}_ssz:{3}_versionHeader:{4}")
+  @ParameterizedTest(name = "version:{0}_blinded:{1}_ssz:{3}")
   @MethodSource("postBlockCases")
-  void shouldReturnOkPostDeneb(
-      final Version version,
-      final boolean isBlindedBlock,
-      final String route,
-      final boolean useSsz,
-      final Optional<String> versionHeader)
+  void postDeneb(
+      final Version version, final boolean isBlindedBlock, final String route, final boolean useSsz)
       throws IOException {
     startRestAPIAtGenesis(SpecMilestone.DENEB);
     dataStructureUtil = new DataStructureUtil(spec);
@@ -144,7 +134,14 @@ void shouldReturnOkPostDeneb(
     prepareResponse(request, version);
 
     postRequestAndAssert(
-        route, request, signedBlockContainerSchema, versionHeader, useSsz, version);
+        route,
+        request,
+        signedBlockContainerSchema,
+        Optional.of(SpecMilestone.DENEB.name()),
+        useSsz,
+        version);
+    postRequestAndAssert(
+        route, request, signedBlockContainerSchema, Optional.empty(), useSsz, version);
   }
 
   private void prepareResponse(final SignedBeaconBlock request, final Version version) {
@@ -207,7 +204,14 @@ private <T extends SszData> void postRequestAndAssert(
               JsonUtil.serialize(request, signedBlockContainerSchema.getJsonTypeDefinition()),
               params,
               versionHeader)) {
-        assertThat(response.code()).isEqualTo(SC_OK);
+        if (version == V2 && versionHeader.isEmpty()) {
+          // the version header is required in V2 APIs
+          assertThat(response.code()).isEqualTo(SC_BAD_REQUEST);
+        } else {
+          // the version header is not required for V1 APIs, the header selector should fall back to
+          // the slot selector
+          assertThat(response.code()).isEqualTo(SC_OK);
+        }
       }
     }
   }

diff --git a/...est/java/tech/pegasys/teku/beaconrestapi/v2/beacon/PostAttestationsV2IntegrationTest.java b/...est/java/tech/pegasys/teku/beaconrestapi/v2/beacon/PostAttestationsV2IntegrationTest.java
@@ -31,7 +31,6 @@
 import tech.pegasys.teku.beaconrestapi.AbstractDataBackedRestAPIIntegrationTest;
 import tech.pegasys.teku.beaconrestapi.handlers.v2.beacon.PostAttestationsV2;
 import tech.pegasys.teku.infrastructure.async.SafeFuture;
-import tech.pegasys.teku.infrastructure.http.RestApiConstants;
 import tech.pegasys.teku.infrastructure.json.JsonTestUtil;
 import tech.pegasys.teku.infrastructure.json.JsonUtil;
 import tech.pegasys.teku.infrastructure.json.types.SerializableTypeDefinition;
@@ -162,6 +161,6 @@ void shouldFailWhenBadConsensusHeaderValue() throws Exception {
         .isEqualTo(
             String.format(
                 "Invalid value for (%s) header: %s",
-                RestApiConstants.HEADER_CONSENSUS_VERSION, badConsensusHeaderValue));
+                HEADER_CONSENSUS_VERSION, badConsensusHeaderValue));
   }
 }
diff --git a/...java/tech/pegasys/teku/beaconrestapi/v2/beacon/PostAttesterSlashingV2IntegrationTest.java b/...java/tech/pegasys/teku/beaconrestapi/v2/beacon/PostAttesterSlashingV2IntegrationTest.java
@@ -34,7 +34,6 @@
 import tech.pegasys.teku.beaconrestapi.AbstractDataBackedRestAPIIntegrationTest;
 import tech.pegasys.teku.beaconrestapi.handlers.v2.beacon.PostAttesterSlashingV2;
 import tech.pegasys.teku.infrastructure.async.SafeFuture;
-import tech.pegasys.teku.infrastructure.http.RestApiConstants;
 import tech.pegasys.teku.infrastructure.json.JsonTestUtil;
 import tech.pegasys.teku.spec.SpecMilestone;
 import tech.pegasys.teku.spec.TestSpecContext;
@@ -156,6 +155,6 @@ void shouldFailWhenBadConsensusHeaderValue() throws Exception {
         .isEqualTo(
             String.format(
                 "Invalid value for (%s) header: %s",
-                RestApiConstants.HEADER_CONSENSUS_VERSION, badConsensusHeaderValue));
+                HEADER_CONSENSUS_VERSION, badConsensusHeaderValue));
   }
 }
diff --git a/.../java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/MilestoneDependentTypesUtil.java b/.../java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/MilestoneDependentTypesUtil.java
@@ -79,21 +79,16 @@ SerializableOneOfTypeDefinition<T> getMultipleSchemaDefinitionFromMilestone(
     return builder.build();
   }
 
-  public static <T extends SszData> DeserializableTypeDefinition<? extends T> slotBasedSelector(
-      final String json,
-      final SchemaDefinitionCache schemaDefinitionCache,
-      final Function<SchemaDefinitions, SszSchema<? extends T>> getSchema) {
-    final Optional<UInt64> slot =
-        // SignedBeaconBlock
-        getSlot(json, "message", "slot")
-            // SignedBlockContents
-            .or(() -> getSlot(json, "signed_block", "message", "slot"));
-    final SpecMilestone milestone =
-        schemaDefinitionCache.milestoneAtSlot(
-            slot.orElseThrow(() -> new BadRequestException("Could not locate slot in JSON data")));
-    return getSchema
-        .apply(schemaDefinitionCache.getSchemaDefinition(milestone))
-        .getJsonTypeDefinition();
+  public static <T extends SszData>
+      DeserializableTypeDefinition<? extends T> headerBasedSelectorWithSlotFallback(
+          final Map<String, String> headers,
+          final String json,
+          final SchemaDefinitionCache schemaDefinitionCache,
+          final Function<SchemaDefinitions, SszSchema<? extends T>> getSchema) {
+    if (headers.containsKey(HEADER_CONSENSUS_VERSION)) {
+      return headerBasedSelector(headers, schemaDefinitionCache, getSchema);
+    }
+    return slotBasedSelector(json, schemaDefinitionCache, getSchema);
   }
 
   public static <T extends SszData> DeserializableTypeDefinition<? extends T> headerBasedSelector(
@@ -117,6 +112,23 @@ public static <T extends SszData> DeserializableTypeDefinition<? extends T> head
     }
   }
 
+  private static <T extends SszData> DeserializableTypeDefinition<? extends T> slotBasedSelector(
+      final String json,
+      final SchemaDefinitionCache schemaDefinitionCache,
+      final Function<SchemaDefinitions, SszSchema<? extends T>> getSchema) {
+    final Optional<UInt64> slot =
+        // SignedBeaconBlock
+        getSlot(json, "message", "slot")
+            // SignedBlockContents
+            .or(() -> getSlot(json, "signed_block", "message", "slot"));
+    final SpecMilestone milestone =
+        schemaDefinitionCache.milestoneAtSlot(
+            slot.orElseThrow(() -> new BadRequestException("Could not locate slot in JSON data")));
+    return getSchema
+        .apply(schemaDefinitionCache.getSchemaDefinition(milestone))
+        .getJsonTypeDefinition();
+  }
+
   private static Optional<UInt64> getSlot(final String json, final String... path) {
     try {
       return JsonUtil.getAttribute(json, CoreTypes.UINT64_TYPE, path);

diff --git a/...pi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/PostBlindedBlock.java b/...pi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/PostBlindedBlock.java
@@ -15,7 +15,7 @@
 
 import static tech.pegasys.teku.beaconrestapi.BeaconRestApiTypes.ETH_CONSENSUS_VERSION_TYPE;
 import static tech.pegasys.teku.beaconrestapi.handlers.v1.beacon.MilestoneDependentTypesUtil.getSchemaDefinitionForAllSupportedMilestones;
-import static tech.pegasys.teku.beaconrestapi.handlers.v1.beacon.MilestoneDependentTypesUtil.slotBasedSelector;
+import static tech.pegasys.teku.beaconrestapi.handlers.v1.beacon.MilestoneDependentTypesUtil.headerBasedSelectorWithSlotFallback;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_ACCEPTED;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_OK;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_SERVICE_UNAVAILABLE;
@@ -100,7 +100,8 @@ broadcast but a different status code is returned (202). Pre-Bellatrix, this end
                         .milestoneAtSlot(blockContainer.getSlot())
                         .equals(milestone)),
             context ->
-                slotBasedSelector(
+                headerBasedSelectorWithSlotFallback(
+                    context.getHeaders(),
                     context.getBody(),
                     schemaDefinitionCache,
                     SchemaDefinitions::getSignedBlindedBlockContainerSchema),

diff --git a/...onrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/PostBlock.java b/...onrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/PostBlock.java
@@ -15,7 +15,7 @@
 
 import static tech.pegasys.teku.beaconrestapi.BeaconRestApiTypes.ETH_CONSENSUS_VERSION_TYPE;
 import static tech.pegasys.teku.beaconrestapi.handlers.v1.beacon.MilestoneDependentTypesUtil.getSchemaDefinitionForAllSupportedMilestones;
-import static tech.pegasys.teku.beaconrestapi.handlers.v1.beacon.MilestoneDependentTypesUtil.slotBasedSelector;
+import static tech.pegasys.teku.beaconrestapi.handlers.v1.beacon.MilestoneDependentTypesUtil.headerBasedSelectorWithSlotFallback;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_ACCEPTED;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_OK;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_SERVICE_UNAVAILABLE;
@@ -100,7 +100,8 @@ validation, a separate success response code (202) is used to indicate that the
                         .milestoneAtSlot(blockContainer.getSlot())
                         .equals(milestone)),
             context ->
-                slotBasedSelector(
+                headerBasedSelectorWithSlotFallback(
+                    context.getHeaders(),
                     context.getBody(),
                     schemaDefinitionCache,
                     SchemaDefinitions::getSignedBlockContainerSchema),

diff --git a/.../src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/PostBlindedBlockV2.java b/.../src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/PostBlindedBlockV2.java
@@ -16,7 +16,7 @@
 import static tech.pegasys.teku.beaconrestapi.BeaconRestApiTypes.ETH_CONSENSUS_VERSION_TYPE;
 import static tech.pegasys.teku.beaconrestapi.BeaconRestApiTypes.PARAMETER_BROADCAST_VALIDATION;
 import static tech.pegasys.teku.beaconrestapi.handlers.v1.beacon.MilestoneDependentTypesUtil.getSchemaDefinitionForAllSupportedMilestones;
-import static tech.pegasys.teku.beaconrestapi.handlers.v1.beacon.MilestoneDependentTypesUtil.slotBasedSelector;
+import static tech.pegasys.teku.beaconrestapi.handlers.v1.beacon.MilestoneDependentTypesUtil.headerBasedSelector;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_ACCEPTED;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_OK;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_SERVICE_UNAVAILABLE;
@@ -113,8 +113,8 @@ broadcast but a different status code is returned (202). Pre-Bellatrix, this end
                         .milestoneAtSlot(blockContainer.getSlot())
                         .equals(milestone)),
             context ->
-                slotBasedSelector(
-                    context.getBody(),
+                headerBasedSelector(
+                    context.getHeaders(),
                     schemaDefinitionCache,
                     SchemaDefinitions::getSignedBlindedBlockContainerSchema),
             spec::deserializeSignedBlindedBlockContainer)

diff --git a/...restapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/PostBlockV2.java b/...restapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/PostBlockV2.java
@@ -16,7 +16,7 @@
 import static tech.pegasys.teku.beaconrestapi.BeaconRestApiTypes.ETH_CONSENSUS_VERSION_TYPE;
 import static tech.pegasys.teku.beaconrestapi.BeaconRestApiTypes.PARAMETER_BROADCAST_VALIDATION;
 import static tech.pegasys.teku.beaconrestapi.handlers.v1.beacon.MilestoneDependentTypesUtil.getSchemaDefinitionForAllSupportedMilestones;
-import static tech.pegasys.teku.beaconrestapi.handlers.v1.beacon.MilestoneDependentTypesUtil.slotBasedSelector;
+import static tech.pegasys.teku.beaconrestapi.handlers.v1.beacon.MilestoneDependentTypesUtil.headerBasedSelector;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_ACCEPTED;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_OK;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_SERVICE_UNAVAILABLE;
@@ -113,8 +113,8 @@ validation, a separate success response code (202) is used to indicate that the
                         .milestoneAtSlot(blockContainer.getSlot())
                         .equals(milestone)),
             context ->
-                slotBasedSelector(
-                    context.getBody(),
+                headerBasedSelector(
+                    context.getHeaders(),
                     schemaDefinitionCache,
                     SchemaDefinitions::getSignedBlockContainerSchema),
             spec::deserializeSignedBlockContainer)

diff --git a/...src/test/java/tech/pegasys/teku/beaconrestapi/schema/MilestoneDependentTypesUtilTest.java b/...src/test/java/tech/pegasys/teku/beaconrestapi/schema/MilestoneDependentTypesUtilTest.java
@@ -19,7 +19,11 @@
 
 import java.util.Collections;
 import java.util.Map;
+import java.util.stream.Stream;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import tech.pegasys.teku.api.exceptions.BadRequestException;
 import tech.pegasys.teku.beaconrestapi.handlers.v1.beacon.MilestoneDependentTypesUtil;
 import tech.pegasys.teku.infrastructure.json.types.DeserializableTypeDefinition;
@@ -33,34 +37,20 @@ public class MilestoneDependentTypesUtilTest {
   private final Spec spec = TestSpecFactory.createMinimalElectra();
   private final SchemaDefinitionCache cache = new SchemaDefinitionCache(spec);
 
-  @Test
-  void headerSelector_UsesConsensusVersionPhase0() {
+  @ParameterizedTest(name = "{0}")
+  @MethodSource("milestones")
+  void headerSelector_UsesConsensusVersionPhase0(final SpecMilestone targetMilestone) {
     final DeserializableTypeDefinition<?> typeDefinition =
         MilestoneDependentTypesUtil.headerBasedSelector(
-            Map.of(HEADER_CONSENSUS_VERSION, SpecMilestone.PHASE0.toString()),
+            Map.of(HEADER_CONSENSUS_VERSION, targetMilestone.toString()),
             cache,
             SchemaDefinitions::getAttestationSchema);
-    assertThat(typeDefinition.getTypeName()).contains("AttestationPhase0");
-  }
-
-  @Test
-  void headerSelector_UsesConsensusVersionDeneb() {
-    final DeserializableTypeDefinition<?> typeDefinition =
-        MilestoneDependentTypesUtil.headerBasedSelector(
-            Map.of(HEADER_CONSENSUS_VERSION, SpecMilestone.DENEB.toString()),
-            cache,
-            SchemaDefinitions::getAttestationSchema);
-    assertThat(typeDefinition.getTypeName()).contains("AttestationPhase0");
-  }
-
-  @Test
-  void headerSelector_UsesConsensusVersionElectra() {
-    final DeserializableTypeDefinition<?> typeDefinition =
-        MilestoneDependentTypesUtil.headerBasedSelector(
-            Map.of(HEADER_CONSENSUS_VERSION, SpecMilestone.ELECTRA.toString()),
-            cache,
-            SchemaDefinitions::getAttestationSchema);
-    assertThat(typeDefinition.getTypeName()).contains("AttestationElectra");
+    assertThat(typeDefinition.getTypeName())
+        .contains(
+            spec.forMilestone(targetMilestone)
+                .getSchemaDefinitions()
+                .getAttestationSchema()
+                .getContainerName());
   }
 
   @Test
@@ -84,4 +74,11 @@ void headerSelector_errorsWhenMissing() {
         .isInstanceOf(BadRequestException.class)
         .hasMessageContaining("Missing required header value for (Eth-Consensus-Version)");
   }
+
+  private static Stream<Arguments> milestones() {
+    return Stream.of(
+        Arguments.of(SpecMilestone.PHASE0),
+        Arguments.of(SpecMilestone.DENEB),
+        Arguments.of(SpecMilestone.ELECTRA));
+  }
 }