As per feedback

Consensys · Oct 23, 2024 · 9f1823c · 9f1823c
1 parent 2477431
commit 9f1823c
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 21 deletions.
diff --git a/.../validator/src/main/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandler.java b/.../validator/src/main/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandler.java
@@ -122,7 +122,7 @@ public class ValidatorApiHandler implements ValidatorApiChannel {
  */
  private static final int DUTY_EPOCH_TOLERANCE = 1;
 
- private final Map<UInt64, Bytes32> createdBlockRootsBySlotCache =
+ private final Map<UInt64, BlockContainerAndMetaData> createdBlocksBySlotCache =
  LimitedMap.createSynchronizedLRU(2);
 
  private final BlockProductionAndPublishingPerformanceFactory
@@ -336,6 +336,9 @@ public SafeFuture<Optional<BlockContainerAndMetaData>> createUnsignedBlock(
  final BLSSignature randaoReveal,
  final Optional<Bytes32> graffiti,
  final Optional<UInt64> requestedBuilderBoostFactor) {
+ if (createdBlocksBySlotCache.containsKey(slot)) {
+ return SafeFuture.completedFuture(Optional.of(createdBlocksBySlotCache.get(slot)));
+ }
  LOG.info("Creating unsigned block for slot {}", slot);
  performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(slot));
  if (isSyncActive()) {
@@ -391,8 +394,7 @@ private SafeFuture<Optional<BlockContainerAndMetaData>> createBlock(
  blockProductionPerformance)
  .thenApply(
  block -> {
- final Bytes32 blockRoot = block.blockContainer().getBlock().getRoot();
- createdBlockRootsBySlotCache.put(slot, blockRoot);
+ createdBlocksBySlotCache.put(slot, block);
  return Optional.of(block);
  });
  }
@@ -869,7 +871,11 @@ private List<ProposerDuty> getProposalSlotsForEpoch(final BeaconState state, fin
  private boolean isLocallyCreatedBlock(final SignedBlockContainer blockContainer) {
  final Bytes32 blockRoot = blockContainer.getSignedBlock().getMessage().getRoot();
  final Bytes32 locallyCreatedBlockRoot =
- createdBlockRootsBySlotCache.get(blockContainer.getSlot());
+ createdBlocksBySlotCache
+ .get(blockContainer.getSlot())
+ .blockContainer()
+ .getBlock()
+ .getRoot();
  return Objects.equals(blockRoot, locallyCreatedBlockRoot);
  }
 

diff --git a/...idator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java b/...idator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java
@@ -543,10 +543,20 @@ public void createUnsignedBlock_shouldCreateBlock() {
  // even if passing a non-empty requestedBlinded and requestedBuilderBoostFactor isn't a valid
  // combination,
  // we still want to check that all parameters are passed down the line to the block factory
- final SafeFuture<Optional<BlockContainerAndMetaData>> result =
+ SafeFuture<Optional<BlockContainerAndMetaData>> result =
  validatorApiHandler.createUnsignedBlock(
  newSlot, randaoReveal, Optional.empty(), Optional.of(ONE));
 
+ assertThat(result).isCompletedWithValue(Optional.of(blockContainerAndMetaData));
+
+ // further calls in the same slot should return the same block
+ result =
+ validatorApiHandler.createUnsignedBlock(
+ newSlot, randaoReveal, Optional.empty(), Optional.of(ONE));
+
+ assertThat(result).isCompletedWithValue(Optional.of(blockContainerAndMetaData));
+
+ // only produced once
  verify(blockFactory)
  .createUnsignedBlock(
  blockSlotState,
@@ -555,7 +565,6 @@ public void createUnsignedBlock_shouldCreateBlock() {
  Optional.empty(),
  Optional.of(ONE),
  BlockProductionPerformance.NOOP);
- assertThat(result).isCompletedWithValue(Optional.of(blockContainerAndMetaData));
  }
 
  @Test

diff --git a/...a/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerBlockProductionManagerImpl.java b/...a/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerBlockProductionManagerImpl.java
@@ -67,16 +67,16 @@ public ExecutionPayloadResult initiateBlockProduction(
  final boolean attemptBuilderFlow,
  final Optional<UInt64> requestedBuilderBoostFactor,
  final BlockProductionPerformance blockProductionPerformance) {
- return executionResultCache.computeIfAbsent(
-  blockSlotState.getSlot(),
-  __ -> {
- if (attemptBuilderFlow) {
- return executeBuilderFlow(
-  context, blockSlotState, requestedBuilderBoostFactor, blockProductionPerformance);
-  } else {
-  return executeLocalFlow(context, blockSlotState, blockProductionPerformance);
-  }
-  });
+ final ExecutionPayloadResult result;
+ if (attemptBuilderFlow) {
+ result =
+ executeBuilderFlow(
+  context, blockSlotState, requestedBuilderBoostFactor, blockProductionPerformance);
+ } else {
+ result = executeLocalFlow(context, blockSlotState, blockProductionPerformance);
+ }
+ executionResultCache.put(blockSlotState.getSlot(), result);
+ return result;
  }
 
  @Override
@@ -88,15 +88,11 @@ public Optional<ExecutionPayloadResult> getCachedPayloadResult(final UInt64 slot
  public SafeFuture<BuilderPayloadOrFallbackData> getUnblindedPayload(
  final SignedBeaconBlock signedBeaconBlock,
  final BlockPublishingPerformance blockPublishingPerformance) {
- final UInt64 slot = signedBeaconBlock.getSlot();
- if (builderResultCache.containsKey(slot)) {
- return SafeFuture.completedFuture(builderResultCache.get(slot));
- }
  return executionLayerChannel
  .builderGetPayload(signedBeaconBlock, this::getCachedPayloadResult)
  .thenPeek(
  builderPayloadOrFallbackData ->
- builderResultCache.put(slot, builderPayloadOrFallbackData))
+ builderResultCache.put(signedBeaconBlock.getSlot(), builderPayloadOrFallbackData))
  .alwaysRun(blockPublishingPerformance::builderGetPayload);
  }