From ba7d4683cc0a58ae888bad6d2f6eb2d0ff80dbef Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Wed, 27 Nov 2024 10:24:12 +0000 Subject: [PATCH 1/5] Track produced block when builder doesn't reveal the payload --- .../publisher/AbstractBlockPublisher.java | 15 ++++++++++++++- .../publisher/AbstractBlockPublisherTest.java | 14 ++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java index 93b39d9bf74..7fb5df7fc4d 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java @@ -18,12 +18,14 @@ import com.google.common.base.Suppliers; import java.util.List; +import java.util.concurrent.TimeoutException; import java.util.function.Supplier; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import tech.pegasys.teku.ethereum.performance.trackers.BlockPublishingPerformance; import tech.pegasys.teku.infrastructure.async.AsyncRunner; import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.infrastructure.exceptions.ExceptionUtil; import tech.pegasys.teku.networking.eth2.gossip.BlockGossipChannel; import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar; import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; @@ -75,7 +77,18 @@ public SafeFuture sendSignedBlock( final BlockPublishingPerformance blockPublishingPerformance) { return blockFactory .unblindSignedBlockIfBlinded(blockContainer.getSignedBlock(), blockPublishingPerformance) - .thenPeek(performanceTracker::saveProducedBlock) + .whenComplete( + (block, ex) -> { + if (ex != null) { + // in case of relay API timeouts when unblinding, we still want to assume the block + // as produced, since the relay could have published it + if (ExceptionUtil.hasCause(ex, TimeoutException.class)) { + performanceTracker.saveProducedBlock(blockContainer.getSignedBlock()); + } + } else { + performanceTracker.saveProducedBlock(block); + } + }) .thenCompose( // creating blob sidecars after unblinding the block to ensure in the blinded flow we // will have the cached builder payload diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java index 50391494269..aeb0a152435 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java @@ -23,6 +23,7 @@ import static tech.pegasys.teku.infrastructure.async.SafeFutureAssert.assertThatSafeFuture; import java.util.List; +import java.util.concurrent.TimeoutException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import tech.pegasys.teku.ethereum.performance.trackers.BlockPublishingPerformance; @@ -252,6 +253,19 @@ public void sendSignedBlock_shouldReturnNotImportedWhenBlockImportFails() { verify(blockPublisher).importBlobSidecars(blobSidecars, BlockPublishingPerformance.NOOP); } + @Test + public void sendSignedBlock_shouldTrackBlockAsProducedIfUnblindingTimeouts() { + when(blockFactory.unblindSignedBlockIfBlinded(signedBlock, BlockPublishingPerformance.NOOP)) + .thenReturn(SafeFuture.failedFuture(new TimeoutException())); + + blockPublisher.sendSignedBlock( + signedBlockContents, + BroadcastValidationLevel.NOT_REQUIRED, + BlockPublishingPerformance.NOOP); + + verify(performanceTracker).saveProducedBlock(signedBlockContents.getSignedBlock()); + } + private SafeFuture prepareBlockImportResult( final BlockImportResult blockImportResult) { return SafeFuture.completedFuture( From 7c892afd8d4f690d11ceb69439a65cb988244b42 Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Wed, 27 Nov 2024 10:51:16 +0000 Subject: [PATCH 2/5] fix assemble --- .../coordinator/publisher/AbstractBlockPublisherTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java index aeb0a152435..c0402dc959c 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java @@ -254,6 +254,7 @@ public void sendSignedBlock_shouldReturnNotImportedWhenBlockImportFails() { } @Test + @SuppressWarnings("FutureReturnValueIgnored") public void sendSignedBlock_shouldTrackBlockAsProducedIfUnblindingTimeouts() { when(blockFactory.unblindSignedBlockIfBlinded(signedBlock, BlockPublishingPerformance.NOOP)) .thenReturn(SafeFuture.failedFuture(new TimeoutException())); From 590f37bf9811aae35477540cd6cb4363271a7e54 Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Thu, 28 Nov 2024 10:21:39 +0000 Subject: [PATCH 3/5] Change --- .../publisher/AbstractBlockPublisher.java | 22 ++++++------------- .../publisher/AbstractBlockPublisherTest.java | 5 ++--- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java index 7fb5df7fc4d..2675fc3e3d9 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java @@ -18,14 +18,12 @@ import com.google.common.base.Suppliers; import java.util.List; -import java.util.concurrent.TimeoutException; import java.util.function.Supplier; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import tech.pegasys.teku.ethereum.performance.trackers.BlockPublishingPerformance; import tech.pegasys.teku.infrastructure.async.AsyncRunner; import tech.pegasys.teku.infrastructure.async.SafeFuture; -import tech.pegasys.teku.infrastructure.exceptions.ExceptionUtil; import tech.pegasys.teku.networking.eth2.gossip.BlockGossipChannel; import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar; import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; @@ -77,18 +75,6 @@ public SafeFuture sendSignedBlock( final BlockPublishingPerformance blockPublishingPerformance) { return blockFactory .unblindSignedBlockIfBlinded(blockContainer.getSignedBlock(), blockPublishingPerformance) - .whenComplete( - (block, ex) -> { - if (ex != null) { - // in case of relay API timeouts when unblinding, we still want to assume the block - // as produced, since the relay could have published it - if (ExceptionUtil.hasCause(ex, TimeoutException.class)) { - performanceTracker.saveProducedBlock(blockContainer.getSignedBlock()); - } - } else { - performanceTracker.saveProducedBlock(block); - } - }) .thenCompose( // creating blob sidecars after unblinding the block to ensure in the blinded flow we // will have the cached builder payload @@ -98,7 +84,13 @@ public SafeFuture sendSignedBlock( Suppliers.memoize(() -> blockFactory.createBlobSidecars(blockContainer)), broadcastValidationLevel, blockPublishingPerformance)) - .thenCompose(result -> calculateResult(blockContainer, result, blockPublishingPerformance)); + .thenCompose(result -> calculateResult(blockContainer, result, blockPublishingPerformance)) + .alwaysRun( + () -> + // always track the block as produced even in case of publishing failures (e.g. + // relay API timeouts during unblinding), because we later do comparison against the + // chain data anyway + performanceTracker.saveProducedBlock(blockContainer.getSignedBlock())); } private SafeFuture diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java index c0402dc959c..7d2f1abd7fb 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java @@ -23,7 +23,6 @@ import static tech.pegasys.teku.infrastructure.async.SafeFutureAssert.assertThatSafeFuture; import java.util.List; -import java.util.concurrent.TimeoutException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import tech.pegasys.teku.ethereum.performance.trackers.BlockPublishingPerformance; @@ -255,9 +254,9 @@ public void sendSignedBlock_shouldReturnNotImportedWhenBlockImportFails() { @Test @SuppressWarnings("FutureReturnValueIgnored") - public void sendSignedBlock_shouldTrackBlockAsProducedIfUnblindingTimeouts() { + public void sendSignedBlock_shouldTrackBlockAsProducedEvenIfExceptionOccurs() { when(blockFactory.unblindSignedBlockIfBlinded(signedBlock, BlockPublishingPerformance.NOOP)) - .thenReturn(SafeFuture.failedFuture(new TimeoutException())); + .thenReturn(SafeFuture.failedFuture(new RuntimeException("oopsy"))); blockPublisher.sendSignedBlock( signedBlockContents, From fcb7b9f356bf540875482ff55ade65fa20312929 Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Thu, 28 Nov 2024 10:38:52 +0000 Subject: [PATCH 4/5] Do tracking at beginning --- .../DefaultPerformanceTracker.java | 6 ++--- .../performance/NoOpPerformanceTracker.java | 4 ++-- .../performance/PerformanceTracker.java | 4 ++-- .../publisher/AbstractBlockPublisher.java | 12 ++++------ .../DefaultPerformanceTrackerTest.java | 23 ++++++++++++------- .../publisher/AbstractBlockPublisherTest.java | 3 ++- 6 files changed, 29 insertions(+), 23 deletions(-) diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTracker.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTracker.java index eb9ea756cae..afce203d35f 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTracker.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTracker.java @@ -420,11 +420,11 @@ public void saveProducedAttestation(final Attestation attestation) { } @Override - public void saveProducedBlock(final SignedBeaconBlock block) { - final UInt64 epoch = spec.computeEpochAtSlot(block.getSlot()); + public void saveProducedBlock(final SlotAndBlockRoot slotAndBlockRoot) { + final UInt64 epoch = spec.computeEpochAtSlot(slotAndBlockRoot.getSlot()); final Set blocksInEpoch = producedBlocksByEpoch.computeIfAbsent(epoch, __ -> concurrentSet()); - blocksInEpoch.add(block.getSlotAndBlockRoot()); + blocksInEpoch.add(slotAndBlockRoot); } @Override diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/NoOpPerformanceTracker.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/NoOpPerformanceTracker.java index 69e28006dcc..988b07a8f62 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/NoOpPerformanceTracker.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/NoOpPerformanceTracker.java @@ -15,7 +15,7 @@ import it.unimi.dsi.fastutil.ints.IntSet; import tech.pegasys.teku.infrastructure.unsigned.UInt64; -import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; +import tech.pegasys.teku.spec.datastructures.blocks.SlotAndBlockRoot; import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.versions.altair.SyncCommitteeMessage; @@ -28,7 +28,7 @@ public void start(final UInt64 nodeStartSlot) {} public void saveProducedAttestation(final Attestation attestation) {} @Override - public void saveProducedBlock(final SignedBeaconBlock block) {} + public void saveProducedBlock(final SlotAndBlockRoot slotAndBlockRoot) {} @Override public void reportBlockProductionAttempt(final UInt64 epoch) {} diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/PerformanceTracker.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/PerformanceTracker.java index 955d3d45216..58aa7785781 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/PerformanceTracker.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/PerformanceTracker.java @@ -16,7 +16,7 @@ import it.unimi.dsi.fastutil.ints.IntSet; import tech.pegasys.teku.ethereum.events.SlotEventsChannel; import tech.pegasys.teku.infrastructure.unsigned.UInt64; -import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; +import tech.pegasys.teku.spec.datastructures.blocks.SlotAndBlockRoot; import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.versions.altair.SyncCommitteeMessage; @@ -26,7 +26,7 @@ public interface PerformanceTracker extends SlotEventsChannel { void saveProducedAttestation(Attestation attestation); - void saveProducedBlock(SignedBeaconBlock block); + void saveProducedBlock(SlotAndBlockRoot slotAndBlockRoot); void reportBlockProductionAttempt(UInt64 epoch); diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java index 2675fc3e3d9..d4df310a2d1 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java @@ -73,6 +73,10 @@ public SafeFuture sendSignedBlock( final SignedBlockContainer blockContainer, final BroadcastValidationLevel broadcastValidationLevel, final BlockPublishingPerformance blockPublishingPerformance) { + // always track the block as produced even in case of publishing failures (e.g. + // relay API timeouts during unblinding), because we later do comparison against the chain data + // anyway + performanceTracker.saveProducedBlock(blockContainer.getSignedBlock().getSlotAndBlockRoot()); return blockFactory .unblindSignedBlockIfBlinded(blockContainer.getSignedBlock(), blockPublishingPerformance) .thenCompose( @@ -84,13 +88,7 @@ public SafeFuture sendSignedBlock( Suppliers.memoize(() -> blockFactory.createBlobSidecars(blockContainer)), broadcastValidationLevel, blockPublishingPerformance)) - .thenCompose(result -> calculateResult(blockContainer, result, blockPublishingPerformance)) - .alwaysRun( - () -> - // always track the block as produced even in case of publishing failures (e.g. - // relay API timeouts during unblinding), because we later do comparison against the - // chain data anyway - performanceTracker.saveProducedBlock(blockContainer.getSignedBlock())); + .thenCompose(result -> calculateResult(blockContainer, result, blockPublishingPerformance)); } private SafeFuture diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTrackerTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTrackerTest.java index 10bfb8bcc15..147f15568c8 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTrackerTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTrackerTest.java @@ -90,8 +90,10 @@ void shouldDisplayPerfectBlockInclusion() { chainUpdater.updateBestBlock(chainUpdater.advanceChainUntil(10)); performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(UInt64.valueOf(1))); performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(UInt64.valueOf(2))); - performanceTracker.saveProducedBlock(chainUpdater.chainBuilder.getBlockAtSlot(1)); - performanceTracker.saveProducedBlock(chainUpdater.chainBuilder.getBlockAtSlot(2)); + performanceTracker.saveProducedBlock( + chainUpdater.chainBuilder.getBlockAtSlot(1).getSlotAndBlockRoot()); + performanceTracker.saveProducedBlock( + chainUpdater.chainBuilder.getBlockAtSlot(2).getSlotAndBlockRoot()); performanceTracker.onSlot(spec.computeStartSlotAtEpoch(UInt64.ONE)); BlockPerformance expectedBlockPerformance = new BlockPerformance(UInt64.ZERO, 2, 2, 2); verify(log).performance(expectedBlockPerformance.toString()); @@ -103,7 +105,7 @@ void shouldDisplayBlockInclusionWhenProducedBlockIsChainHead() { final SignedBlockAndState bestBlock = chainUpdater.advanceChainUntil(2); chainUpdater.updateBestBlock(bestBlock); performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(bestBlock.getSlot())); - performanceTracker.saveProducedBlock(bestBlock.getBlock()); + performanceTracker.saveProducedBlock(bestBlock.getBlock().getSlotAndBlockRoot()); performanceTracker.onSlot(lastSlot); BlockPerformance expectedBlockPerformance = new BlockPerformance(UInt64.ZERO, 1, 1, 1); verify(log).performance(expectedBlockPerformance.toString()); @@ -115,9 +117,12 @@ void shouldDisplayOneMissedBlock() { performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(UInt64.valueOf(1))); performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(UInt64.valueOf(2))); performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(UInt64.valueOf(3))); - performanceTracker.saveProducedBlock(chainUpdater.chainBuilder.getBlockAtSlot(1)); - performanceTracker.saveProducedBlock(chainUpdater.chainBuilder.getBlockAtSlot(2)); - performanceTracker.saveProducedBlock(dataStructureUtil.randomSignedBeaconBlock(3)); + performanceTracker.saveProducedBlock( + chainUpdater.chainBuilder.getBlockAtSlot(1).getSlotAndBlockRoot()); + performanceTracker.saveProducedBlock( + chainUpdater.chainBuilder.getBlockAtSlot(2).getSlotAndBlockRoot()); + performanceTracker.saveProducedBlock( + dataStructureUtil.randomSignedBeaconBlock(3).getSlotAndBlockRoot()); performanceTracker.onSlot(spec.computeStartSlotAtEpoch(UInt64.ONE)); BlockPerformance expectedBlockPerformance = new BlockPerformance(UInt64.ZERO, 3, 2, 3); verify(log).performance(expectedBlockPerformance.toString()); @@ -258,8 +263,10 @@ void shouldClearOldSentObjects() { chainUpdater.updateBestBlock(chainUpdater.advanceChainUntil(10)); performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(UInt64.valueOf(1))); performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(UInt64.valueOf(2))); - performanceTracker.saveProducedBlock(chainUpdater.chainBuilder.getBlockAtSlot(1)); - performanceTracker.saveProducedBlock(chainUpdater.chainBuilder.getBlockAtSlot(2)); + performanceTracker.saveProducedBlock( + chainUpdater.chainBuilder.getBlockAtSlot(1).getSlotAndBlockRoot()); + performanceTracker.saveProducedBlock( + chainUpdater.chainBuilder.getBlockAtSlot(2).getSlotAndBlockRoot()); performanceTracker.saveProducedAttestation( spec.getGenesisSchemaDefinitions() .getAttestationSchema() diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java index 7d2f1abd7fb..986ddcaaa3e 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java @@ -263,7 +263,8 @@ public void sendSignedBlock_shouldTrackBlockAsProducedEvenIfExceptionOccurs() { BroadcastValidationLevel.NOT_REQUIRED, BlockPublishingPerformance.NOOP); - verify(performanceTracker).saveProducedBlock(signedBlockContents.getSignedBlock()); + verify(performanceTracker) + .saveProducedBlock(signedBlockContents.getSignedBlock().getSlotAndBlockRoot()); } private SafeFuture prepareBlockImportResult( From bdceade133a2a1453e4ac61b3863288dabe138e8 Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Thu, 28 Nov 2024 10:59:06 +0000 Subject: [PATCH 5/5] Move tracking of produced block to ValidatorApiHandler --- .../ValidatorApiHandlerIntegrationTest.java | 1 - .../coordinator/ValidatorApiHandler.java | 6 ++++++ .../publisher/AbstractBlockPublisher.java | 8 ------- .../publisher/BlockPublisherDeneb.java | 3 --- .../publisher/BlockPublisherPhase0.java | 3 --- .../MilestoneBasedBlockPublisher.java | 4 ---- .../coordinator/ValidatorApiHandlerTest.java | 5 +++++ .../publisher/AbstractBlockPublisherTest.java | 21 ------------------- .../publisher/BlockPublisherDenebTest.java | 2 -- .../publisher/BlockPublisherPhase0Test.java | 2 -- .../beaconchain/BeaconChainController.java | 1 - 11 files changed, 11 insertions(+), 45 deletions(-) diff --git a/beacon/validator/src/integrationTest/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerIntegrationTest.java b/beacon/validator/src/integrationTest/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerIntegrationTest.java index 85a683ef102..0f658707b33 100644 --- a/beacon/validator/src/integrationTest/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerIntegrationTest.java +++ b/beacon/validator/src/integrationTest/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerIntegrationTest.java @@ -198,7 +198,6 @@ public void setup(final SpecContext specContext) { blockGossipChannel, blockBlobSidecarsTrackersPool, blobSidecarGossipChannel, - performanceTracker, dutyMetrics, P2PConfig.DEFAULT_GOSSIP_BLOBS_AFTER_BLOCK_ENABLED)); } diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandler.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandler.java index 46c84df772b..04648dd04fb 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandler.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandler.java @@ -364,6 +364,12 @@ public SafeFuture> createUnsignedBlockIntern requestedBuilderBoostFactor, blockSlotState, blockProductionPerformance)) + .thenPeek( + maybeBlock -> + maybeBlock.ifPresent( + block -> + performanceTracker.saveProducedBlock( + block.blockContainer().getBlock().getSlotAndBlockRoot()))) .alwaysRun(blockProductionPerformance::complete); } diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java index d4df310a2d1..dd1b6490522 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java @@ -36,7 +36,6 @@ import tech.pegasys.teku.validator.api.SendSignedBlockResult; import tech.pegasys.teku.validator.coordinator.BlockFactory; import tech.pegasys.teku.validator.coordinator.DutyMetrics; -import tech.pegasys.teku.validator.coordinator.performance.PerformanceTracker; public abstract class AbstractBlockPublisher implements BlockPublisher { private static final Logger LOG = LogManager.getLogger(); @@ -48,7 +47,6 @@ public abstract class AbstractBlockPublisher implements BlockPublisher { protected final BlockFactory blockFactory; protected final BlockImportChannel blockImportChannel; protected final BlockGossipChannel blockGossipChannel; - protected final PerformanceTracker performanceTracker; protected final DutyMetrics dutyMetrics; public AbstractBlockPublisher( @@ -56,14 +54,12 @@ public AbstractBlockPublisher( final BlockFactory blockFactory, final BlockGossipChannel blockGossipChannel, final BlockImportChannel blockImportChannel, - final PerformanceTracker performanceTracker, final DutyMetrics dutyMetrics, final boolean gossipBlobsAfterBlock) { this.asyncRunner = asyncRunner; this.blockFactory = blockFactory; this.blockImportChannel = blockImportChannel; this.blockGossipChannel = blockGossipChannel; - this.performanceTracker = performanceTracker; this.dutyMetrics = dutyMetrics; this.gossipBlobsAfterBlock = gossipBlobsAfterBlock; } @@ -73,10 +69,6 @@ public SafeFuture sendSignedBlock( final SignedBlockContainer blockContainer, final BroadcastValidationLevel broadcastValidationLevel, final BlockPublishingPerformance blockPublishingPerformance) { - // always track the block as produced even in case of publishing failures (e.g. - // relay API timeouts during unblinding), because we later do comparison against the chain data - // anyway - performanceTracker.saveProducedBlock(blockContainer.getSignedBlock().getSlotAndBlockRoot()); return blockFactory .unblindSignedBlockIfBlinded(blockContainer.getSignedBlock(), blockPublishingPerformance) .thenCompose( diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherDeneb.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherDeneb.java index d7f054ac629..6350e6b06d9 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherDeneb.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherDeneb.java @@ -24,7 +24,6 @@ import tech.pegasys.teku.statetransition.block.BlockImportChannel; import tech.pegasys.teku.validator.coordinator.BlockFactory; import tech.pegasys.teku.validator.coordinator.DutyMetrics; -import tech.pegasys.teku.validator.coordinator.performance.PerformanceTracker; public class BlockPublisherDeneb extends BlockPublisherPhase0 { @@ -38,7 +37,6 @@ public BlockPublisherDeneb( final BlockGossipChannel blockGossipChannel, final BlockBlobSidecarsTrackersPool blockBlobSidecarsTrackersPool, final BlobSidecarGossipChannel blobSidecarGossipChannel, - final PerformanceTracker performanceTracker, final DutyMetrics dutyMetrics, final boolean gossipBlobsAfterBlock) { super( @@ -46,7 +44,6 @@ public BlockPublisherDeneb( blockFactory, blockGossipChannel, blockImportChannel, - performanceTracker, dutyMetrics, gossipBlobsAfterBlock); this.blockBlobSidecarsTrackersPool = blockBlobSidecarsTrackersPool; diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherPhase0.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherPhase0.java index 014417b21ba..ad1925151fe 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherPhase0.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherPhase0.java @@ -25,7 +25,6 @@ import tech.pegasys.teku.statetransition.block.BlockImportChannel.BlockImportAndBroadcastValidationResults; import tech.pegasys.teku.validator.coordinator.BlockFactory; import tech.pegasys.teku.validator.coordinator.DutyMetrics; -import tech.pegasys.teku.validator.coordinator.performance.PerformanceTracker; public class BlockPublisherPhase0 extends AbstractBlockPublisher { @@ -34,7 +33,6 @@ public BlockPublisherPhase0( final BlockFactory blockFactory, final BlockGossipChannel blockGossipChannel, final BlockImportChannel blockImportChannel, - final PerformanceTracker performanceTracker, final DutyMetrics dutyMetrics, final boolean gossipBlobsAfterBlock) { super( @@ -42,7 +40,6 @@ public BlockPublisherPhase0( blockFactory, blockGossipChannel, blockImportChannel, - performanceTracker, dutyMetrics, gossipBlobsAfterBlock); } diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/MilestoneBasedBlockPublisher.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/MilestoneBasedBlockPublisher.java index e1cc2024a53..b0d9393f8b6 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/MilestoneBasedBlockPublisher.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/MilestoneBasedBlockPublisher.java @@ -31,7 +31,6 @@ import tech.pegasys.teku.validator.api.SendSignedBlockResult; import tech.pegasys.teku.validator.coordinator.BlockFactory; import tech.pegasys.teku.validator.coordinator.DutyMetrics; -import tech.pegasys.teku.validator.coordinator.performance.PerformanceTracker; public class MilestoneBasedBlockPublisher implements BlockPublisher { @@ -47,7 +46,6 @@ public MilestoneBasedBlockPublisher( final BlockGossipChannel blockGossipChannel, final BlockBlobSidecarsTrackersPool blockBlobSidecarsTrackersPool, final BlobSidecarGossipChannel blobSidecarGossipChannel, - final PerformanceTracker performanceTracker, final DutyMetrics dutyMetrics, final boolean gossipBlobsAfterBlock) { this.spec = spec; @@ -57,7 +55,6 @@ public MilestoneBasedBlockPublisher( blockFactory, blockGossipChannel, blockImportChannel, - performanceTracker, dutyMetrics, gossipBlobsAfterBlock); @@ -72,7 +69,6 @@ public MilestoneBasedBlockPublisher( blockGossipChannel, blockBlobSidecarsTrackersPool, blobSidecarGossipChannel, - performanceTracker, dutyMetrics, gossipBlobsAfterBlock)); diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java index 346eee64205..d882571c41d 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java @@ -531,6 +531,11 @@ public void createUnsignedBlock_shouldCreateBlock() { Optional.empty(), Optional.of(ONE), BlockProductionPerformance.NOOP); + + verify(performanceTracker).reportBlockProductionAttempt(spec.computeEpochAtSlot(newSlot)); + verify(performanceTracker) + .saveProducedBlock( + blockContainerAndMetaData.blockContainer().getBlock().getSlotAndBlockRoot()); } @Test diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java index 986ddcaaa3e..e80dd2a3d98 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java @@ -45,7 +45,6 @@ import tech.pegasys.teku.validator.api.SendSignedBlockResult; import tech.pegasys.teku.validator.coordinator.BlockFactory; import tech.pegasys.teku.validator.coordinator.DutyMetrics; -import tech.pegasys.teku.validator.coordinator.performance.PerformanceTracker; public class AbstractBlockPublisherTest { private final StubAsyncRunner asyncRunner = new StubAsyncRunner(); @@ -54,7 +53,6 @@ public class AbstractBlockPublisherTest { private final BlockFactory blockFactory = mock(BlockFactory.class); private final BlockGossipChannel blockGossipChannel = mock(BlockGossipChannel.class); private final BlockImportChannel blockImportChannel = mock(BlockImportChannel.class); - private final PerformanceTracker performanceTracker = mock(PerformanceTracker.class); private final DutyMetrics dutyMetrics = mock(DutyMetrics.class); private final AbstractBlockPublisher blockPublisher = @@ -64,7 +62,6 @@ public class AbstractBlockPublisherTest { blockFactory, blockGossipChannel, blockImportChannel, - performanceTracker, dutyMetrics, false)); @@ -195,7 +192,6 @@ public void sendSignedBlock_shouldPublishBlobsAfterBlockWhenOptionIsEnabled() { blockFactory, blockGossipChannel, blockImportChannel, - performanceTracker, dutyMetrics, true)); @@ -252,21 +248,6 @@ public void sendSignedBlock_shouldReturnNotImportedWhenBlockImportFails() { verify(blockPublisher).importBlobSidecars(blobSidecars, BlockPublishingPerformance.NOOP); } - @Test - @SuppressWarnings("FutureReturnValueIgnored") - public void sendSignedBlock_shouldTrackBlockAsProducedEvenIfExceptionOccurs() { - when(blockFactory.unblindSignedBlockIfBlinded(signedBlock, BlockPublishingPerformance.NOOP)) - .thenReturn(SafeFuture.failedFuture(new RuntimeException("oopsy"))); - - blockPublisher.sendSignedBlock( - signedBlockContents, - BroadcastValidationLevel.NOT_REQUIRED, - BlockPublishingPerformance.NOOP); - - verify(performanceTracker) - .saveProducedBlock(signedBlockContents.getSignedBlock().getSlotAndBlockRoot()); - } - private SafeFuture prepareBlockImportResult( final BlockImportResult blockImportResult) { return SafeFuture.completedFuture( @@ -288,7 +269,6 @@ public BlockPublisherTest( final BlockFactory blockFactory, final BlockGossipChannel blockGossipChannel, final BlockImportChannel blockImportChannel, - final PerformanceTracker performanceTracker, final DutyMetrics dutyMetrics, final boolean gossipBlobsAfterBlock) { super( @@ -296,7 +276,6 @@ public BlockPublisherTest( blockFactory, blockGossipChannel, blockImportChannel, - performanceTracker, dutyMetrics, gossipBlobsAfterBlock); } diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherDenebTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherDenebTest.java index b2e3d9f270c..a1eaf55edaa 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherDenebTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherDenebTest.java @@ -32,7 +32,6 @@ import tech.pegasys.teku.statetransition.block.BlockImportChannel; import tech.pegasys.teku.validator.coordinator.BlockFactory; import tech.pegasys.teku.validator.coordinator.DutyMetrics; -import tech.pegasys.teku.validator.coordinator.performance.PerformanceTracker; class BlockPublisherDenebTest { private final BlockBlobSidecarsTrackersPool blockBlobSidecarsTrackersPool = @@ -47,7 +46,6 @@ class BlockPublisherDenebTest { mock(BlockGossipChannel.class), blockBlobSidecarsTrackersPool, blobSidecarGossipChannel, - mock(PerformanceTracker.class), mock(DutyMetrics.class), true); diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherPhase0Test.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherPhase0Test.java index a5c95d61471..230d33e2142 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherPhase0Test.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/BlockPublisherPhase0Test.java @@ -29,7 +29,6 @@ import tech.pegasys.teku.statetransition.block.BlockImportChannel; import tech.pegasys.teku.validator.coordinator.BlockFactory; import tech.pegasys.teku.validator.coordinator.DutyMetrics; -import tech.pegasys.teku.validator.coordinator.performance.PerformanceTracker; class BlockPublisherPhase0Test { private final BlockGossipChannel blockGossipChannel = mock(BlockGossipChannel.class); @@ -41,7 +40,6 @@ class BlockPublisherPhase0Test { mock(BlockFactory.class), blockGossipChannel, blockImportChannel, - mock(PerformanceTracker.class), mock(DutyMetrics.class), false); diff --git a/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java b/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java index d57ef3bb489..5ceac99ab19 100644 --- a/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java +++ b/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java @@ -981,7 +981,6 @@ public void initValidatorApiHandler() { blockGossipChannel, blockBlobSidecarsTrackersPool, blobSidecarGossipChannel, - performanceTracker, dutyMetrics, beaconConfig.p2pConfig().isGossipBlobsAfterBlockEnabled());