diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/BlockProductionDuty.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/BlockProductionDuty.java index dcb03d50faa..1df14137889 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/BlockProductionDuty.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/BlockProductionDuty.java @@ -77,22 +77,51 @@ public SafeFuture performDuty() { return forkProvider.getForkInfo(slot).thenCompose(this::produceBlock); } - public SafeFuture produceBlock(final ForkInfo forkInfo) { + private SafeFuture produceBlock(final ForkInfo forkInfo) { return createRandaoReveal(forkInfo) .thenCompose( signature -> validatorDutyMetrics.record( () -> createUnsignedBlock(signature), this, Step.CREATE)) + .thenCompose(this::validateBlock) .thenCompose( - unsignedBlock -> + blockContainer -> validatorDutyMetrics.record( - () -> signBlockContainer(forkInfo, unsignedBlock), this, Step.SIGN)) + () -> signBlockContainer(forkInfo, blockContainer), this, Step.SIGN)) .thenCompose( signedBlockContainer -> validatorDutyMetrics.record(() -> sendBlock(signedBlockContainer), this, Step.SEND)) .exceptionally(error -> DutyResult.forError(validator.getPublicKey(), error)); } + private SafeFuture createRandaoReveal(final ForkInfo forkInfo) { + return validator.getSigner().createRandaoReveal(spec.computeEpochAtSlot(slot), forkInfo); + } + + private SafeFuture> createUnsignedBlock( + final BLSSignature randaoReveal) { + return validatorApiChannel.createUnsignedBlock( + slot, randaoReveal, validator.getGraffiti(), useBlindedBlock); + } + + private SafeFuture validateBlock( + final Optional maybeBlockContainer) { + final BlockContainer unsignedBlockContainer = + maybeBlockContainer.orElseThrow( + () -> new IllegalStateException("Node was not syncing but could not create block")); + checkArgument( + unsignedBlockContainer.getSlot().equals(slot), + "Unsigned block slot (%s) does not match expected slot %s", + unsignedBlockContainer.getSlot(), + slot); + return SafeFuture.completedFuture(unsignedBlockContainer); + } + + private SafeFuture signBlockContainer( + final ForkInfo forkInfo, final BlockContainer blockContainer) { + return blockContainerSigner.sign(blockContainer, validator, forkInfo); + } + private SafeFuture sendBlock(final SignedBlockContainer signedBlockContainer) { return validatorApiChannel .sendSignedBlock(signedBlockContainer) @@ -111,28 +140,6 @@ private SafeFuture sendBlock(final SignedBlockContainer signedBlockC }); } - public SafeFuture> createUnsignedBlock(final BLSSignature randaoReveal) { - return validatorApiChannel.createUnsignedBlock( - slot, randaoReveal, validator.getGraffiti(), useBlindedBlock); - } - - public SafeFuture createRandaoReveal(final ForkInfo forkInfo) { - return validator.getSigner().createRandaoReveal(spec.computeEpochAtSlot(slot), forkInfo); - } - - public SafeFuture signBlockContainer( - final ForkInfo forkInfo, final Optional maybeBlockContainer) { - final BlockContainer unsignedBlockContainer = - maybeBlockContainer.orElseThrow( - () -> new IllegalStateException("Node was not syncing but could not create block")); - checkArgument( - unsignedBlockContainer.getSlot().equals(slot), - "Unsigned block slot (%s) does not match expected slot %s", - unsignedBlockContainer.getSlot(), - slot); - return blockContainerSigner.sign(unsignedBlockContainer, validator, forkInfo); - } - @Override public String toString() { return "BlockProductionDuty{" 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 e240e68a5a4..cd8cc921f72 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 @@ -17,7 +17,9 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import static tech.pegasys.teku.infrastructure.async.SafeFuture.completedFuture; @@ -69,6 +71,7 @@ import tech.pegasys.teku.validator.api.ValidatorApiChannel; import tech.pegasys.teku.validator.client.ForkProvider; import tech.pegasys.teku.validator.client.Validator; +import tech.pegasys.teku.validator.client.duties.ValidatorDutyMetrics.Step; import tech.pegasys.teku.validator.client.signer.BlockContainerSigner; import tech.pegasys.teku.validator.client.signer.MilestoneBasedBlockContainerSigner; @@ -92,9 +95,8 @@ class BlockProductionDutyTest { private final ValidatorLogger validatorLogger = mock(ValidatorLogger.class); private final BlockContainerSigner blockContainerSigner = new MilestoneBasedBlockContainerSigner(spec); - - private final StubMetricsSystem metricSystem = new StubMetricsSystem(); - + private final ValidatorDutyMetrics validatorDutyMetrics = + spy(ValidatorDutyMetrics.create(new StubMetricsSystem())); private BlockProductionDuty duty; @BeforeEach @@ -108,7 +110,7 @@ public void setUp() { blockContainerSigner, false, spec, - ValidatorDutyMetrics.create(metricSystem)); + validatorDutyMetrics); when(forkProvider.getForkInfo(any())).thenReturn(completedFuture(fork)); } @@ -124,7 +126,7 @@ public void shouldCreateAndPublishBlock(final boolean isBlindedBlocksEnabled) { blockContainerSigner, isBlindedBlocksEnabled, spec, - ValidatorDutyMetrics.create(metricSystem)); + validatorDutyMetrics); final BLSSignature randaoReveal = dataStructureUtil.randomSignature(); final BLSSignature blockSignature = dataStructureUtil.randomSignature(); final BeaconBlock unsignedBlock; @@ -157,6 +159,10 @@ public void shouldCreateAndPublishBlock(final boolean isBlindedBlocksEnabled) { eq(Set.of(unsignedBlock.hashTreeRoot())), ArgumentMatchers.argThat(Optional::isPresent)); verifyNoMoreInteractions(validatorLogger); + + verify(validatorDutyMetrics).record(any(), any(BlockProductionDuty.class), eq(Step.CREATE)); + verify(validatorDutyMetrics).record(any(), any(BlockProductionDuty.class), eq(Step.SIGN)); + verify(validatorDutyMetrics).record(any(), any(BlockProductionDuty.class), eq(Step.SEND)); } @Test @@ -170,7 +176,7 @@ public void forDeneb_shouldCreateAndPublishBlockContents() { blockContainerSigner, false, spec, - ValidatorDutyMetrics.create(metricSystem)); + validatorDutyMetrics); final BLSSignature randaoReveal = dataStructureUtil.randomSignature(); final BLSSignature blockSignature = dataStructureUtil.randomSignature(); @@ -238,6 +244,10 @@ public void forDeneb_shouldCreateAndPublishBlockContents() { assertThat(signedBlobSidecar.getSignature()) .isEqualTo(blobSidecarsSignatures.get(unsignedBlobSidecar)); }); + + verify(validatorDutyMetrics).record(any(), any(BlockProductionDuty.class), eq(Step.CREATE)); + verify(validatorDutyMetrics).record(any(), any(BlockProductionDuty.class), eq(Step.SIGN)); + verify(validatorDutyMetrics).record(any(), any(BlockProductionDuty.class), eq(Step.SEND)); } @Test @@ -251,7 +261,7 @@ public void forDeneb_shouldCreateAndPublishBlindedBlockContents() { blockContainerSigner, true, spec, - ValidatorDutyMetrics.create(metricSystem)); + validatorDutyMetrics); final BLSSignature randaoReveal = dataStructureUtil.randomSignature(); final BLSSignature blockSignature = dataStructureUtil.randomSignature(); @@ -326,6 +336,10 @@ public void forDeneb_shouldCreateAndPublishBlindedBlockContents() { assertThat(signedBlindedBlobSidecar.getSignature()) .isEqualTo(blindedBlobSidecarsSignatures.get(unsignedBlindedBlobSidecar)); }); + + verify(validatorDutyMetrics).record(any(), any(BlockProductionDuty.class), eq(Step.CREATE)); + verify(validatorDutyMetrics).record(any(), any(BlockProductionDuty.class), eq(Step.SIGN)); + verify(validatorDutyMetrics).record(any(), any(BlockProductionDuty.class), eq(Step.SEND)); } @Test @@ -335,6 +349,7 @@ public void shouldFailWhenCreateRandaoFails() { .thenReturn(failedFuture(error)); assertDutyFails(error); + verifyNoInteractions(validatorDutyMetrics); } @Test @@ -364,6 +379,9 @@ public void shouldFailWhenCreateUnsignedBlockFails() { .thenReturn(failedFuture(error)); assertDutyFails(error); + + verify(validatorDutyMetrics).record(any(), any(BlockProductionDuty.class), eq(Step.CREATE)); + verifyNoMoreInteractions(validatorDutyMetrics); } @Test @@ -384,6 +402,9 @@ public void shouldFailWhenCreateUnsignedBlockReturnsEmpty() { eq(Set.of(validator.getPublicKey().toAbbreviatedString())), any(IllegalStateException.class)); verifyNoMoreInteractions(validatorLogger); + + verify(validatorDutyMetrics).record(any(), any(BlockProductionDuty.class), eq(Step.CREATE)); + verifyNoMoreInteractions(validatorDutyMetrics); } @Test @@ -399,6 +420,10 @@ public void shouldFailWhenSigningBlockFails() { when(signer.signBlock(unsignedBlock, fork)).thenReturn(failedFuture(error)); assertDutyFails(error); + + verify(validatorDutyMetrics).record(any(), any(BlockProductionDuty.class), eq(Step.CREATE)); + verify(validatorDutyMetrics).record(any(), any(BlockProductionDuty.class), eq(Step.SIGN)); + verifyNoMoreInteractions(validatorDutyMetrics); } public void assertDutyFails(final RuntimeException error) {