From 35da85a5a23b3eb62dcc6eabfdb58a9af75cf1b8 Mon Sep 17 00:00:00 2001 From: Mehdi AOUADI Date: Thu, 22 Aug 2024 15:19:52 +0200 Subject: [PATCH] use header based selector with slot fallback (#8522) * use header based selector with slot fallback --- .../PostBlindedAndUnblindedBlockTest.java | 66 ++++++++++--------- .../PostAttestationsV2IntegrationTest.java | 3 +- ...PostAttesterSlashingV2IntegrationTest.java | 3 +- .../beacon/MilestoneDependentTypesUtil.java | 42 +++++++----- .../handlers/v1/beacon/PostBlindedBlock.java | 5 +- .../handlers/v1/beacon/PostBlock.java | 5 +- .../v2/beacon/PostBlindedBlockV2.java | 6 +- .../handlers/v2/beacon/PostBlockV2.java | 6 +- .../MilestoneDependentTypesUtilTest.java | 45 ++++++------- 9 files changed, 97 insertions(+), 84 deletions(-) diff --git a/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v1/validator/PostBlindedAndUnblindedBlockTest.java b/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v1/validator/PostBlindedAndUnblindedBlockTest.java index 52408aa2409..6b85496b015 100644 --- a/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v1/validator/PostBlindedAndUnblindedBlockTest.java +++ b/data/beaconrestapi/src/integration-test/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 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 versionHeader = (Optional) 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 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 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 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/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v2/beacon/PostAttestationsV2IntegrationTest.java b/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v2/beacon/PostAttestationsV2IntegrationTest.java index 0abfb89421e..24205d46ecf 100644 --- a/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v2/beacon/PostAttestationsV2IntegrationTest.java +++ b/data/beaconrestapi/src/integration-test/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/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v2/beacon/PostAttesterSlashingV2IntegrationTest.java b/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v2/beacon/PostAttesterSlashingV2IntegrationTest.java index 04e787abc3c..2058bc6b00b 100644 --- a/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v2/beacon/PostAttesterSlashingV2IntegrationTest.java +++ b/data/beaconrestapi/src/integration-test/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/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/MilestoneDependentTypesUtil.java b/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/MilestoneDependentTypesUtil.java index 7f290f6fa79..c6d4efea5ea 100644 --- a/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/MilestoneDependentTypesUtil.java +++ b/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/MilestoneDependentTypesUtil.java @@ -79,21 +79,16 @@ SerializableOneOfTypeDefinition getMultipleSchemaDefinitionFromMilestone( return builder.build(); } - public static DeserializableTypeDefinition slotBasedSelector( - final String json, - final SchemaDefinitionCache schemaDefinitionCache, - final Function> getSchema) { - final Optional 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 + DeserializableTypeDefinition headerBasedSelectorWithSlotFallback( + final Map headers, + final String json, + final SchemaDefinitionCache schemaDefinitionCache, + final Function> getSchema) { + if (headers.containsKey(HEADER_CONSENSUS_VERSION)) { + return headerBasedSelector(headers, schemaDefinitionCache, getSchema); + } + return slotBasedSelector(json, schemaDefinitionCache, getSchema); } public static DeserializableTypeDefinition headerBasedSelector( @@ -117,6 +112,23 @@ public static DeserializableTypeDefinition head } } + private static DeserializableTypeDefinition slotBasedSelector( + final String json, + final SchemaDefinitionCache schemaDefinitionCache, + final Function> getSchema) { + final Optional 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 getSlot(final String json, final String... path) { try { return JsonUtil.getAttribute(json, CoreTypes.UINT64_TYPE, path); diff --git a/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/PostBlindedBlock.java b/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/PostBlindedBlock.java index 20bef8e8967..0eed8674133 100644 --- a/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/PostBlindedBlock.java +++ b/data/beaconrestapi/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/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/PostBlock.java b/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/PostBlock.java index afddb792a92..41ee4d05a82 100644 --- a/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/PostBlock.java +++ b/data/beaconrestapi/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/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/PostBlindedBlockV2.java b/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/PostBlindedBlockV2.java index 5fe6da80e54..af6f0e894c3 100644 --- a/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/PostBlindedBlockV2.java +++ b/data/beaconrestapi/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/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/PostBlockV2.java b/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/PostBlockV2.java index 78200d40670..c07c6a6ede2 100644 --- a/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/PostBlockV2.java +++ b/data/beaconrestapi/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/data/beaconrestapi/src/test/java/tech/pegasys/teku/beaconrestapi/schema/MilestoneDependentTypesUtilTest.java b/data/beaconrestapi/src/test/java/tech/pegasys/teku/beaconrestapi/schema/MilestoneDependentTypesUtilTest.java index d01d72cc7dc..3441c7894bf 100644 --- a/data/beaconrestapi/src/test/java/tech/pegasys/teku/beaconrestapi/schema/MilestoneDependentTypesUtilTest.java +++ b/data/beaconrestapi/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 milestones() { + return Stream.of( + Arguments.of(SpecMilestone.PHASE0), + Arguments.of(SpecMilestone.DENEB), + Arguments.of(SpecMilestone.ELECTRA)); + } }