From e57a27918ca5114847e15aa857267530fc1e5f2f Mon Sep 17 00:00:00 2001 From: Paul Harris Date: Sat, 19 Oct 2024 07:57:41 +1000 Subject: [PATCH 1/2] Eth-Consensus-version header now passed for json block production from VC When producing a block, JSON payloads require the Eth-Consensus-version header. Generally using SSZ this was correct, but there was no test case for JSON block production, and it wasn't providing the header correctly. Because probably 99% of our block production is via SSZ, this was not really visible. This would not be a problem for local BN+VC, but if the teku VC ever falls back to json (or is instructed to produce JSON), block production could potentially fail if the BN is following spec. Changed an AT to check both SSZ and JSON block production. fixes #8753 Signed-off-by: Paul Harris --- CHANGELOG.md | 1 + .../acceptance/BlockProposalAcceptanceTest.java | 9 ++++++--- .../acceptance/dsl/TekuNodeConfigBuilder.java | 6 ++++++ .../handlers/SendSignedBlockRequest.java | 17 +++++++++-------- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1e20bc9d74..aff65a07716 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,3 +12,4 @@ - Clean up old beacon states when switching from ARCHIVE to PRUNE or MINIMAL data storage mode ### Bug Fixes + - Fixed a block production issue for Validator Client (vc), where required headers were not provided for JSON payloads. diff --git a/acceptance-tests/src/acceptance-test/java/tech/pegasys/teku/test/acceptance/BlockProposalAcceptanceTest.java b/acceptance-tests/src/acceptance-test/java/tech/pegasys/teku/test/acceptance/BlockProposalAcceptanceTest.java index 11bddd0fdfc..762a10959b8 100644 --- a/acceptance-tests/src/acceptance-test/java/tech/pegasys/teku/test/acceptance/BlockProposalAcceptanceTest.java +++ b/acceptance-tests/src/acceptance-test/java/tech/pegasys/teku/test/acceptance/BlockProposalAcceptanceTest.java @@ -21,7 +21,8 @@ import java.util.Arrays; import java.util.Locale; import org.apache.tuweni.bytes.Bytes32; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import tech.pegasys.teku.infrastructure.bytes.Bytes20; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.test.acceptance.dsl.AcceptanceTestBase; @@ -34,8 +35,9 @@ public class BlockProposalAcceptanceTest extends AcceptanceTestBase { private static final URL JWT_FILE = Resources.getResource("auth/ee-jwt-secret.hex"); - @Test - void shouldHaveCorrectFeeRecipientAndGraffiti() throws Exception { + @ParameterizedTest(name = "ssz_encode={0}") + @ValueSource(booleans = {true, false}) + void shouldHaveCorrectFeeRecipientAndGraffiti(final boolean useSszBlocks) throws Exception { final String networkName = "swift"; final ValidatorKeystores validatorKeystores = @@ -69,6 +71,7 @@ void shouldHaveCorrectFeeRecipientAndGraffiti() throws Exception { .withValidatorProposerDefaultFeeRecipient(defaultFeeRecipient) .withInteropModeDisabled() .withBeaconNodes(beaconNode) + .withBeaconNodeSszBlocksEnabled(useSszBlocks) .withGraffiti(userGraffiti) .withNetwork("auto") .build()); diff --git a/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuNodeConfigBuilder.java b/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuNodeConfigBuilder.java index b931a600a78..5ad12f151e5 100644 --- a/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuNodeConfigBuilder.java +++ b/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuNodeConfigBuilder.java @@ -530,6 +530,12 @@ public TekuNodeConfigBuilder withValidatorProposerDefaultFeeRecipient( return this; } + public TekuNodeConfigBuilder withBeaconNodeSszBlocksEnabled(final boolean enabled) { + LOG.debug("beacon-node-ssz-blocks-enabled={}", enabled); + configMap.put("beacon-node-ssz-blocks-enabled", enabled); + return this; + } + public TekuNodeConfigBuilder withStartupTargetPeerCount(final int startupTargetPeerCount) { mustBe(NodeType.BEACON_NODE); LOG.debug("Xstartup-target-peer-count={}", startupTargetPeerCount); diff --git a/validator/remote/src/main/java/tech/pegasys/teku/validator/remote/typedef/handlers/SendSignedBlockRequest.java b/validator/remote/src/main/java/tech/pegasys/teku/validator/remote/typedef/handlers/SendSignedBlockRequest.java index c36cac6aefa..ac28dc1d66e 100644 --- a/validator/remote/src/main/java/tech/pegasys/teku/validator/remote/typedef/handlers/SendSignedBlockRequest.java +++ b/validator/remote/src/main/java/tech/pegasys/teku/validator/remote/typedef/handlers/SendSignedBlockRequest.java @@ -72,25 +72,25 @@ public SendSignedBlockResult submit( blinded ? schemaDefinitions.getSignedBlindedBlockContainerSchema().getJsonTypeDefinition() : schemaDefinitions.getSignedBlockContainerSchema().getJsonTypeDefinition(); - + final SpecMilestone milestone = spec.atSlot(signedBlockContainer.getSlot()).getMilestone(); return preferSszBlockEncoding.get() ? sendSignedBlockAsSszOrFallback( - apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition) + apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition, milestone) : sendSignedBlockAsJson( - apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition); + apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition, milestone); } private SendSignedBlockResult sendSignedBlockAsSszOrFallback( final ValidatorApiMethod apiMethod, final SignedBlockContainer signedBlockContainer, final BroadcastValidationLevel broadcastValidationLevel, - final DeserializableTypeDefinition typeDefinition) { - final SpecMilestone milestone = spec.atSlot(signedBlockContainer.getSlot()).getMilestone(); + final DeserializableTypeDefinition typeDefinition, + final SpecMilestone milestone) { final SendSignedBlockResult result = sendSignedBlockAsSsz(apiMethod, signedBlockContainer, broadcastValidationLevel, milestone); if (!result.isPublished() && !preferSszBlockEncoding.get()) { return sendSignedBlockAsJson( - apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition); + apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition, milestone); } return result; } @@ -117,14 +117,15 @@ private SendSignedBlockResult sendSignedBlockAsJson( final ValidatorApiMethod apiMethod, final SignedBlockContainer signedBlockContainer, final BroadcastValidationLevel broadcastValidationLevel, - final DeserializableTypeDefinition typeDefinition) { + final DeserializableTypeDefinition typeDefinition, + final SpecMilestone milestone) { return postJson( apiMethod, emptyMap(), Map.of( PARAM_BROADCAST_VALIDATION, broadcastValidationLevel.name().toLowerCase(Locale.ROOT)), - emptyMap(), + Map.of(HEADER_CONSENSUS_VERSION, milestone.name().toLowerCase(Locale.ROOT)), signedBlockContainer, typeDefinition, new ResponseHandler<>()) From aa71434e5587a007ca00d3facd75c35cd7aaaa11 Mon Sep 17 00:00:00 2001 From: Paul Harris Date: Sat, 19 Oct 2024 17:05:18 +1000 Subject: [PATCH 2/2] refactored privates a little to reduce header definition duplication, and elaborated on changelog. Signed-off-by: Paul Harris --- CHANGELOG.md | 2 +- .../handlers/SendSignedBlockRequest.java | 22 ++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aff65a07716..63d1428b0c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,4 +12,4 @@ - Clean up old beacon states when switching from ARCHIVE to PRUNE or MINIMAL data storage mode ### Bug Fixes - - Fixed a block production issue for Validator Client (vc), where required headers were not provided for JSON payloads. + - Fixed a block production issue for Validator Client (24.10.0 to 24.10.2 teku VC), where required headers were not provided for JSON payloads. Default SSZ block production was unaffected. The required header is in the beacon-api spec but was not updated in all places for the VC. diff --git a/validator/remote/src/main/java/tech/pegasys/teku/validator/remote/typedef/handlers/SendSignedBlockRequest.java b/validator/remote/src/main/java/tech/pegasys/teku/validator/remote/typedef/handlers/SendSignedBlockRequest.java index ac28dc1d66e..bb70aa7ef79 100644 --- a/validator/remote/src/main/java/tech/pegasys/teku/validator/remote/typedef/handlers/SendSignedBlockRequest.java +++ b/validator/remote/src/main/java/tech/pegasys/teku/validator/remote/typedef/handlers/SendSignedBlockRequest.java @@ -68,16 +68,18 @@ public SendSignedBlockResult submit( final SchemaDefinitions schemaDefinitions = spec.atSlot(signedBlockContainer.getSlot()).getSchemaDefinitions(); + final SpecMilestone milestone = spec.atSlot(signedBlockContainer.getSlot()).getMilestone(); + final Map headers = + Map.of(HEADER_CONSENSUS_VERSION, milestone.name().toLowerCase(Locale.ROOT)); final DeserializableTypeDefinition typeDefinition = blinded ? schemaDefinitions.getSignedBlindedBlockContainerSchema().getJsonTypeDefinition() : schemaDefinitions.getSignedBlockContainerSchema().getJsonTypeDefinition(); - final SpecMilestone milestone = spec.atSlot(signedBlockContainer.getSlot()).getMilestone(); return preferSszBlockEncoding.get() ? sendSignedBlockAsSszOrFallback( - apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition, milestone) + apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition, headers) : sendSignedBlockAsJson( - apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition, milestone); + apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition, headers); } private SendSignedBlockResult sendSignedBlockAsSszOrFallback( @@ -85,12 +87,12 @@ private SendSignedBlockResult sendSignedBlockAsSszOrFallback( final SignedBlockContainer signedBlockContainer, final BroadcastValidationLevel broadcastValidationLevel, final DeserializableTypeDefinition typeDefinition, - final SpecMilestone milestone) { + final Map headers) { final SendSignedBlockResult result = - sendSignedBlockAsSsz(apiMethod, signedBlockContainer, broadcastValidationLevel, milestone); + sendSignedBlockAsSsz(apiMethod, signedBlockContainer, broadcastValidationLevel, headers); if (!result.isPublished() && !preferSszBlockEncoding.get()) { return sendSignedBlockAsJson( - apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition, milestone); + apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition, headers); } return result; } @@ -99,14 +101,14 @@ private SendSignedBlockResult sendSignedBlockAsSsz( final ValidatorApiMethod apiMethod, final SignedBlockContainer signedBlockContainer, final BroadcastValidationLevel broadcastValidationLevel, - final SpecMilestone milestone) { + final Map headers) { return postOctetStream( apiMethod, emptyMap(), Map.of( PARAM_BROADCAST_VALIDATION, broadcastValidationLevel.name().toLowerCase(Locale.ROOT)), - Map.of(HEADER_CONSENSUS_VERSION, milestone.name().toLowerCase(Locale.ROOT)), + headers, signedBlockContainer.sszSerialize().toArray(), sszResponseHandler) .map(__ -> SendSignedBlockResult.success(signedBlockContainer.getRoot())) @@ -118,14 +120,14 @@ private SendSignedBlockResult sendSignedBlockAsJson( final SignedBlockContainer signedBlockContainer, final BroadcastValidationLevel broadcastValidationLevel, final DeserializableTypeDefinition typeDefinition, - final SpecMilestone milestone) { + final Map headers) { return postJson( apiMethod, emptyMap(), Map.of( PARAM_BROADCAST_VALIDATION, broadcastValidationLevel.name().toLowerCase(Locale.ROOT)), - Map.of(HEADER_CONSENSUS_VERSION, milestone.name().toLowerCase(Locale.ROOT)), + headers, signedBlockContainer, typeDefinition, new ResponseHandler<>())