From d761f5bec12c442b77d7505c9cb3619d0efc8a1c Mon Sep 17 00:00:00 2001 From: Paul Harris Date: Sun, 20 Oct 2024 06:05:36 +1000 Subject: [PATCH] Eth-Consensus-version header now passed for json block production from VC (#8754) 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 + .../BlockProposalAcceptanceTest.java | 9 ++++--- .../acceptance/dsl/TekuNodeConfigBuilder.java | 6 +++++ .../handlers/SendSignedBlockRequest.java | 25 +++++++++++-------- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1e20bc9d74..63d1428b0c6 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 (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/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..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,29 +68,31 @@ 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(); - return preferSszBlockEncoding.get() ? sendSignedBlockAsSszOrFallback( - apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition) + apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition, headers) : sendSignedBlockAsJson( - apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition); + apiMethod, signedBlockContainer, broadcastValidationLevel, typeDefinition, headers); } 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 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); + 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())) @@ -117,14 +119,15 @@ private SendSignedBlockResult sendSignedBlockAsJson( final ValidatorApiMethod apiMethod, final SignedBlockContainer signedBlockContainer, final BroadcastValidationLevel broadcastValidationLevel, - final DeserializableTypeDefinition typeDefinition) { + final DeserializableTypeDefinition typeDefinition, + final Map headers) { return postJson( apiMethod, emptyMap(), Map.of( PARAM_BROADCAST_VALIDATION, broadcastValidationLevel.name().toLowerCase(Locale.ROOT)), - emptyMap(), + headers, signedBlockContainer, typeDefinition, new ResponseHandler<>())