From cdf0c7ac28884662a22e048a6053941aa40d96b6 Mon Sep 17 00:00:00 2001 From: Mehdi AOUADI Date: Wed, 22 Nov 2023 11:52:10 +0100 Subject: [PATCH 1/2] add Deneb block contents signing (#7723) * add deneb block contents signing --- .../signer/BlockContainerSignerDeneb.java | 40 ++++- .../duties/BlockProductionDutyTest.java | 147 +++++++++++++++++- 2 files changed, 176 insertions(+), 11 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/signer/BlockContainerSignerDeneb.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/signer/BlockContainerSignerDeneb.java index 3f278aea401..97fd2d69c9f 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/signer/BlockContainerSignerDeneb.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/signer/BlockContainerSignerDeneb.java @@ -13,14 +13,18 @@ package tech.pegasys.teku.validator.client.signer; -import java.util.Collections; +import java.util.List; import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.infrastructure.ssz.SszCollection; +import tech.pegasys.teku.kzg.KZGProof; import tech.pegasys.teku.spec.Spec; +import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.Blob; import tech.pegasys.teku.spec.datastructures.blocks.BeaconBlock; import tech.pegasys.teku.spec.datastructures.blocks.BlockContainer; import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; import tech.pegasys.teku.spec.datastructures.blocks.SignedBlockContainer; import tech.pegasys.teku.spec.datastructures.state.ForkInfo; +import tech.pegasys.teku.spec.datastructures.type.SszKZGProof; import tech.pegasys.teku.spec.schemas.SchemaDefinitionsDeneb; import tech.pegasys.teku.validator.client.Validator; @@ -44,15 +48,37 @@ public SafeFuture sign( return signBlock(unsignedBlock, validator, forkInfo) .thenApply( signedBlock -> { - // Blinded flow if (signedBlock.isBlinded()) { return signedBlock; + } else { + final List kzgProofs = + unsignedBlockContainer + .getKzgProofs() + .map( + sszKzgProofs -> + sszKzgProofs.asList().stream() + .map(SszKZGProof::getKZGProof) + .toList()) + .orElseThrow( + () -> + new RuntimeException( + String.format( + "Unable to get KZG Proofs when signing Deneb block at slot %d", + unsignedBlockContainer.getSlot().longValue()))); + final List blobs = + unsignedBlockContainer + .getBlobs() + .map(SszCollection::asList) + .orElseThrow( + () -> + new RuntimeException( + String.format( + "Unable to get blobs when signing Deneb block at slot %d", + unsignedBlockContainer.getSlot().longValue()))); + return schemaDefinitions + .getSignedBlockContentsSchema() + .create(signedBlock, kzgProofs, blobs); } - // Unblinded flow - // TODO: add proofs and blobs - return schemaDefinitions - .getSignedBlockContentsSchema() - .create(signedBlock, Collections.emptyList(), Collections.emptyList()); }); } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/duties/BlockProductionDutyTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/duties/BlockProductionDutyTest.java index da462d2f7b1..f1f59018525 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/duties/BlockProductionDutyTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/duties/BlockProductionDutyTest.java @@ -27,6 +27,7 @@ import static tech.pegasys.teku.infrastructure.async.SafeFutureAssert.safeJoin; import static tech.pegasys.teku.infrastructure.unsigned.UInt64.ZERO; +import java.util.List; import java.util.Optional; import java.util.Set; import org.apache.tuweni.bytes.Bytes; @@ -44,9 +45,11 @@ import tech.pegasys.teku.infrastructure.logging.ValidatorLogger; import tech.pegasys.teku.infrastructure.metrics.StubMetricsSystem; import tech.pegasys.teku.infrastructure.metrics.Validator.ValidatorDutyMetricsSteps; +import tech.pegasys.teku.infrastructure.ssz.SszList; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.TestSpecFactory; +import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.Blob; import tech.pegasys.teku.spec.datastructures.blocks.BeaconBlock; import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; import tech.pegasys.teku.spec.datastructures.blocks.SignedBlockContainer; @@ -55,6 +58,7 @@ import tech.pegasys.teku.spec.datastructures.blocks.versions.deneb.SignedBlockContents; import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadSummary; import tech.pegasys.teku.spec.datastructures.state.ForkInfo; +import tech.pegasys.teku.spec.datastructures.type.SszKZGProof; import tech.pegasys.teku.spec.datastructures.validator.BroadcastValidationLevel; import tech.pegasys.teku.spec.signatures.Signer; import tech.pegasys.teku.spec.util.DataStructureUtil; @@ -180,6 +184,10 @@ public void forDeneb_shouldCreateAndPublishBlockContents() { // can create BlockContents only post-Deneb final BlockContents unsignedBlockContents = dataStructureUtil.randomBlockContents(denebSlot); final BeaconBlock unsignedBlock = unsignedBlockContents.getBlock(); + final List blobsFromUnsignedBlockContents = + unsignedBlockContents.getBlobs().orElseThrow().asList(); + final List kzgProofsFromUnsignedBlockContents = + unsignedBlockContents.getKzgProofs().orElseThrow().asList(); when(signer.createRandaoReveal(spec.computeEpochAtSlot(denebSlot), fork)) .thenReturn(completedFuture(randaoReveal)); when(signer.signBlock(unsignedBlockContents.getBlock(), fork)) @@ -218,7 +226,23 @@ public void forDeneb_shouldCreateAndPublishBlockContents() { assertThat(signedBlock.getMessage()).isEqualTo(unsignedBlock); assertThat(signedBlock.getSignature()).isEqualTo(blockSignature); - // TODO Add test for blobs and kzg proofs once added + assertThat(signedBlockContents.getKzgProofs()).isPresent(); + + final SszList kzgProofsFromSignedBlockContent = + signedBlockContents.getKzgProofs().get(); + + assertThat(kzgProofsFromSignedBlockContent).isNotEmpty(); + + assertThat(kzgProofsFromUnsignedBlockContents) + .isEqualTo(kzgProofsFromSignedBlockContent.asList()); + + assertThat(signedBlockContents.getBlobs()).isPresent(); + + final SszList blobsFromSignedBlockContents = signedBlockContents.getBlobs().get(); + + assertThat(blobsFromSignedBlockContents).isNotEmpty(); + + assertThat(blobsFromUnsignedBlockContents).isEqualTo(blobsFromSignedBlockContents.asList()); verify(validatorDutyMetrics) .record(any(), any(BlockProductionDuty.class), eq(ValidatorDutyMetricsSteps.CREATE)); @@ -291,6 +315,90 @@ public void forDeneb_shouldCreateAndPublishBlindedBlock() { .record(any(), any(BlockProductionDuty.class), eq(ValidatorDutyMetricsSteps.SEND)); } + @Test + public void forDeneb_shouldFailWhenNoKzgProofs() { + duty = + new BlockProductionDuty( + validator, + denebSlot, + forkProvider, + validatorApiChannel, + blockContainerSigner, + false, + false, + spec, + validatorDutyMetrics); + + final BLSSignature randaoReveal = dataStructureUtil.randomSignature(); + final BLSSignature blockSignature = dataStructureUtil.randomSignature(); + // can create BlockContents only post-Deneb + final BlockContents unsignedBlockContents = dataStructureUtil.randomBlockContents(denebSlot); + final BlockContents unsignedBlockContentsMock = mock(BlockContents.class); + when(unsignedBlockContentsMock.getKzgProofs()).thenReturn(Optional.empty()); + final Bytes32 blockRoot = dataStructureUtil.randomBytes32(); + when(unsignedBlockContentsMock.getSlot()).thenReturn(unsignedBlockContents.getSlot()); + when(unsignedBlockContentsMock.getBlock()).thenReturn(unsignedBlockContents.getBlock()); + when(signer.createRandaoReveal(spec.computeEpochAtSlot(denebSlot), fork)) + .thenReturn(completedFuture(randaoReveal)); + when(signer.signBlock(unsignedBlockContentsMock.getBlock(), fork)) + .thenReturn(completedFuture(blockSignature)); + when(validatorApiChannel.createUnsignedBlock( + denebSlot, randaoReveal, Optional.of(graffiti), false)) + .thenReturn(completedFuture(Optional.of(unsignedBlockContentsMock))); + when(validatorApiChannel.sendSignedBlock(any(), any())) + .thenReturn(completedFuture(SendSignedBlockResult.success(blockRoot))); + + final RuntimeException error = + new RuntimeException( + String.format( + "Unable to get KZG Proofs when signing Deneb block at slot %d", + denebSlot.longValue())); + + assertDutyFails(error, denebSlot); + } + + @Test + public void forDeneb_shouldFailWhenNoBlobs() { + duty = + new BlockProductionDuty( + validator, + denebSlot, + forkProvider, + validatorApiChannel, + blockContainerSigner, + false, + false, + spec, + validatorDutyMetrics); + + final BLSSignature randaoReveal = dataStructureUtil.randomSignature(); + final BLSSignature blockSignature = dataStructureUtil.randomSignature(); + // can create BlockContents only post-Deneb + final BlockContents unsignedBlockContents = dataStructureUtil.randomBlockContents(denebSlot); + final BlockContents unsignedBlockContentsMock = mock(BlockContents.class); + when(unsignedBlockContentsMock.getBlobs()).thenReturn(Optional.empty()); + when(unsignedBlockContentsMock.getKzgProofs()).thenReturn(unsignedBlockContents.getKzgProofs()); + final Bytes32 blockRoot = dataStructureUtil.randomBytes32(); + when(unsignedBlockContentsMock.getSlot()).thenReturn(unsignedBlockContents.getSlot()); + when(unsignedBlockContentsMock.getBlock()).thenReturn(unsignedBlockContents.getBlock()); + when(signer.createRandaoReveal(spec.computeEpochAtSlot(denebSlot), fork)) + .thenReturn(completedFuture(randaoReveal)); + when(signer.signBlock(unsignedBlockContentsMock.getBlock(), fork)) + .thenReturn(completedFuture(blockSignature)); + when(validatorApiChannel.createUnsignedBlock( + denebSlot, randaoReveal, Optional.of(graffiti), false)) + .thenReturn(completedFuture(Optional.of(unsignedBlockContentsMock))); + when(validatorApiChannel.sendSignedBlock(any(), any())) + .thenReturn(completedFuture(SendSignedBlockResult.success(blockRoot))); + + final RuntimeException error = + new RuntimeException( + String.format( + "Unable to get blobs when signing Deneb block at slot %d", denebSlot.longValue())); + + assertDutyFails(error, denebSlot); + } + @Test public void shouldFailWhenCreateRandaoFails() { final RuntimeException error = new RuntimeException("Sorry!"); @@ -454,6 +562,10 @@ public void forDeneb_shouldUseBlockV3ToCreateAndPublishBlockContents() { // can create BlockContents only post-Deneb final BlockContents unsignedBlockContents = dataStructureUtil.randomBlockContents(denebSlot); final BeaconBlock unsignedBlock = unsignedBlockContents.getBlock(); + final List blobsFromUnsignedBlockContents = + unsignedBlockContents.getBlobs().orElseThrow().asList(); + final List kzgProofsFromUnsignedBlockContents = + unsignedBlockContents.getKzgProofs().orElseThrow().asList(); when(signer.createRandaoReveal(spec.computeEpochAtSlot(denebSlot), fork)) .thenReturn(completedFuture(randaoReveal)); @@ -491,7 +603,23 @@ public void forDeneb_shouldUseBlockV3ToCreateAndPublishBlockContents() { assertThat(signedBlock.getMessage()).isEqualTo(unsignedBlock); assertThat(signedBlock.getSignature()).isEqualTo(blockSignature); - // TODO Add test for blobs and kzg proofs once added + assertThat(signedBlockContents.getKzgProofs()).isPresent(); + + final SszList kzgProofsFromSignedBlockContent = + signedBlockContents.getKzgProofs().get(); + + assertThat(kzgProofsFromSignedBlockContent).isNotEmpty(); + + assertThat(kzgProofsFromUnsignedBlockContents) + .isEqualTo(kzgProofsFromSignedBlockContent.asList()); + + assertThat(signedBlockContents.getBlobs()).isPresent(); + + final SszList blobsFromSignedBlockContents = signedBlockContents.getBlobs().get(); + + assertThat(blobsFromSignedBlockContents).isNotEmpty(); + + assertThat(blobsFromUnsignedBlockContents).isEqualTo(blobsFromSignedBlockContents.asList()); verify(validatorDutyMetrics) .record(any(), any(BlockProductionDuty.class), eq(ValidatorDutyMetricsSteps.CREATE)); @@ -502,10 +630,21 @@ public void forDeneb_shouldUseBlockV3ToCreateAndPublishBlockContents() { } public void assertDutyFails(final RuntimeException error) { - performAndReportDuty(); + assertDutyFails(error, CAPELLA_SLOT); + } + + public void assertDutyFails(final RuntimeException expectedError, final UInt64 slot) { + performAndReportDuty(slot); + final ArgumentCaptor errorCaptor = ArgumentCaptor.forClass(Throwable.class); verify(validatorLogger) .dutyFailed( - TYPE, CAPELLA_SLOT, Set.of(validator.getPublicKey().toAbbreviatedString()), error); + eq(TYPE), + eq(slot), + eq(Set.of(validator.getPublicKey().toAbbreviatedString())), + errorCaptor.capture()); + final Throwable actualError = errorCaptor.getValue(); + assertThat(expectedError.getCause()).isEqualTo(actualError.getCause()); + assertThat(expectedError.getMessage()).isEqualTo(actualError.getMessage()); verifyNoMoreInteractions(validatorLogger); } From 4c20f7871d1c3e050d6bc221ea47571192cc64e5 Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Wed, 22 Nov 2023 14:08:27 +0000 Subject: [PATCH 2/2] Avoid unnecessary ssz conversions when signing block for Deneb (#7747) --- .../blocks/versions/deneb/SignedBlockContents.java | 10 +++++++++- .../versions/deneb/SignedBlockContentsSchema.java | 7 +++++++ .../client/signer/BlockContainerSignerDeneb.java | 14 +++----------- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/versions/deneb/SignedBlockContents.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/versions/deneb/SignedBlockContents.java index 8fe5acceef8..a8ebb3317e2 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/versions/deneb/SignedBlockContents.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/versions/deneb/SignedBlockContents.java @@ -37,7 +37,7 @@ public SignedBlockContents( final SignedBeaconBlock signedBeaconBlock, final List kzgProofs, final List blobs) { - super( + this( schema, signedBeaconBlock, schema @@ -46,6 +46,14 @@ public SignedBlockContents( schema.getBlobsSchema().createFromElements(blobs)); } + public SignedBlockContents( + final SignedBlockContentsSchema schema, + final SignedBeaconBlock signedBeaconBlock, + final SszList kzgProofs, + final SszList blobs) { + super(schema, signedBeaconBlock, kzgProofs, blobs); + } + @Override public SignedBeaconBlock getSignedBlock() { return getField0(); diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/versions/deneb/SignedBlockContentsSchema.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/versions/deneb/SignedBlockContentsSchema.java index 90899c0bc06..34f68ca6540 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/versions/deneb/SignedBlockContentsSchema.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/versions/deneb/SignedBlockContentsSchema.java @@ -68,6 +68,13 @@ public SignedBlockContents create( return new SignedBlockContents(this, signedBeaconBlock, kzgProofs, blobs); } + public SignedBlockContents create( + final SignedBeaconBlock signedBeaconBlock, + final SszList kzgProofs, + final SszList blobs) { + return new SignedBlockContents(this, signedBeaconBlock, kzgProofs, blobs); + } + @Override public SignedBlockContents createFromBackingNode(final TreeNode node) { return new SignedBlockContents(this, node); diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/signer/BlockContainerSignerDeneb.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/signer/BlockContainerSignerDeneb.java index 97fd2d69c9f..aa0648f3f1b 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/signer/BlockContainerSignerDeneb.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/signer/BlockContainerSignerDeneb.java @@ -13,10 +13,8 @@ package tech.pegasys.teku.validator.client.signer; -import java.util.List; import tech.pegasys.teku.infrastructure.async.SafeFuture; -import tech.pegasys.teku.infrastructure.ssz.SszCollection; -import tech.pegasys.teku.kzg.KZGProof; +import tech.pegasys.teku.infrastructure.ssz.SszList; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.Blob; import tech.pegasys.teku.spec.datastructures.blocks.BeaconBlock; @@ -51,24 +49,18 @@ public SafeFuture sign( if (signedBlock.isBlinded()) { return signedBlock; } else { - final List kzgProofs = + final SszList kzgProofs = unsignedBlockContainer .getKzgProofs() - .map( - sszKzgProofs -> - sszKzgProofs.asList().stream() - .map(SszKZGProof::getKZGProof) - .toList()) .orElseThrow( () -> new RuntimeException( String.format( "Unable to get KZG Proofs when signing Deneb block at slot %d", unsignedBlockContainer.getSlot().longValue()))); - final List blobs = + final SszList blobs = unsignedBlockContainer .getBlobs() - .map(SszCollection::asList) .orElseThrow( () -> new RuntimeException(