Eth-Consensus-version header now passed for json block production fro…

…m 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 <[email protected]>
Consensys · Oct 19, 2024 · d761f5b · d761f5b
1 parent b22b3d5
commit d761f5b
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 14 deletions.
diff --git 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/...c/acceptance-test/java/tech/pegasys/teku/test/acceptance/BlockProposalAcceptanceTest.java b/...c/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/...ts/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuNodeConfigBuilder.java b/...ts/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/...main/java/tech/pegasys/teku/validator/remote/typedef/handlers/SendSignedBlockRequest.java b/...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<String, String> headers =
+        Map.of(HEADER_CONSENSUS_VERSION, milestone.name().toLowerCase(Locale.ROOT));
     final DeserializableTypeDefinition<SignedBlockContainer> 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<SignedBlockContainer> typeDefinition) {
-    final SpecMilestone milestone = spec.atSlot(signedBlockContainer.getSlot()).getMilestone();
+      final DeserializableTypeDefinition<SignedBlockContainer> typeDefinition,
+      final Map<String, String> 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<String, String> 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<SignedBlockContainer> typeDefinition) {
+      final DeserializableTypeDefinition<SignedBlockContainer> typeDefinition,
+      final Map<String, String> headers) {
     return postJson(
             apiMethod,
             emptyMap(),
             Map.of(
                 PARAM_BROADCAST_VALIDATION,
                 broadcastValidationLevel.name().toLowerCase(Locale.ROOT)),
-            emptyMap(),
+            headers,
             signedBlockContainer,
             typeDefinition,
             new ResponseHandler<>())
Original file line number	Diff line number	Diff line change
Expand Up		@@ -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.