Caching changes in ExecutionLayerBlockProductionManager

beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactory.java

@@ -465,7 +465,7 @@ public Function<SignedBlockContainer, List<BlobSidecar>> createBlobSidecarsSelec
         // the blobs and the proofs wouldn't be part of the BlockContainer.
         final BuilderPayloadOrFallbackData builderPayloadOrFallbackData =
             executionLayerBlockProductionManager
-                .getCachedUnblindedPayload(block.getSlotAndBlockRoot())
+                .getCachedUnblindedPayload(slot)
                 .orElseThrow(
                     () ->
                         new IllegalStateException(

beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/AbstractBlockFactoryTest.java

@@ -374,8 +374,7 @@ protected BlockAndBlobSidecars createBlockAndBlobSidecars(
     }
 
     // simulate caching of the builder payload
-    when(executionLayer.getCachedUnblindedPayload(
-            signedBlockContainer.getSignedBlock().getSlotAndBlockRoot()))
+    when(executionLayer.getCachedUnblindedPayload(signedBlockContainer.getSlot()))
         .thenReturn(builderPayload.map(BuilderPayloadOrFallbackData::create));
 
     final List<BlobSidecar> blobSidecars =

beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDenebTest.java

@@ -156,7 +156,7 @@ void shouldCreateValidBlobSidecarsForBlindedBlock() {
     final SignedBlockContainer block = blockAndBlobSidecars.block();
     final List<BlobSidecar> blobSidecars = blockAndBlobSidecars.blobSidecars();
 
-    verify(executionLayer).getCachedUnblindedPayload(block.getSlotAndBlockRoot());
+    verify(executionLayer).getCachedUnblindedPayload(block.getSlot());
 
     final SszList<SszKZGCommitment> expectedCommitments =
         block.getSignedBlock().getMessage().getBody().getOptionalBlobKzgCommitments().orElseThrow();

beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java

@@ -50,7 +50,6 @@
 import tech.pegasys.teku.spec.datastructures.blocks.BeaconBlock;
 import tech.pegasys.teku.spec.datastructures.blocks.Eth1Data;
 import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock;
-import tech.pegasys.teku.spec.datastructures.blocks.SlotAndBlockRoot;
 import tech.pegasys.teku.spec.datastructures.blocks.blockbody.BeaconBlockBody;
 import tech.pegasys.teku.spec.datastructures.blocks.blockbody.BeaconBlockBodyBuilder;
 import tech.pegasys.teku.spec.datastructures.blocks.blockbody.BeaconBlockBodySchema;
@@ -818,7 +817,7 @@ void shouldFailCreatingBlobSidecarsIfBuilderBlobsBundleCommitmentsRootIsNotConsi
         dataStructureUtil.randomBuilderBlobsBundle(3);
 
     prepareCachedBuilderPayload(
-        signedBlindedBeaconBlock.getSlotAndBlockRoot(),
+        signedBlindedBeaconBlock.getSlot(),
         dataStructureUtil.randomExecutionPayload(),
         blobsBundle);
 
@@ -843,7 +842,7 @@ void shouldFailCreatingBlobSidecarsIfBuilderBlobsBundleProofsIsNotConsistent() {
     when(blobsBundle.getBlobs()).thenReturn(dataStructureUtil.randomSszBlobs(2));
 
     prepareCachedBuilderPayload(
-        signedBlindedBeaconBlock.getSlotAndBlockRoot(),
+        signedBlindedBeaconBlock.getSlot(),
         dataStructureUtil.randomExecutionPayload(),
         blobsBundle);
 
@@ -868,7 +867,7 @@ void shouldFailCreatingBlobSidecarsIfBuilderBlobsBundleBlobsIsNotConsistent() {
     when(blobsBundle.getProofs()).thenReturn(dataStructureUtil.randomSszKZGProofs(2));
 
     prepareCachedBuilderPayload(
-        signedBlindedBeaconBlock.getSlotAndBlockRoot(),
+        signedBlindedBeaconBlock.getSlot(),
         dataStructureUtil.randomExecutionPayload(),
         blobsBundle);
 
@@ -902,14 +901,10 @@ void shouldCreateBlobSidecarsForBlindedBlock(final boolean useLocalFallback) {
                   .toList(),
               blobsBundle.getProofs().stream().map(SszKZGProof::getKZGProof).toList(),
               blobsBundle.getBlobs().stream().toList());
-      prepareCachedFallbackData(
-          signedBlindedBeaconBlock.getSlotAndBlockRoot(),
-          executionPayload,
-          localFallbackBlobsBundle);
+      prepareCachedFallbackData(slot, executionPayload, localFallbackBlobsBundle);
     } else {
 
-      prepareCachedBuilderPayload(
-          signedBlindedBeaconBlock.getSlotAndBlockRoot(), executionPayload, blobsBundle);
+      prepareCachedBuilderPayload(slot, executionPayload, blobsBundle);
     }
 
     final List<BlobSidecar> blobSidecars =
@@ -1286,23 +1281,20 @@ private void prepareCachedPayloadHeaderWithFallbackResult(
   }
 
   private void prepareCachedBuilderPayload(
-      final SlotAndBlockRoot slotAndBlockRoot,
+      final UInt64 slot,
       final ExecutionPayload executionPayload,
       final tech.pegasys.teku.spec.datastructures.builder.BlobsBundle blobsBundle) {
     final BuilderPayload builderPayload =
-        SchemaDefinitionsDeneb.required(
-                spec.atSlot(slotAndBlockRoot.getSlot()).getSchemaDefinitions())
+        SchemaDefinitionsDeneb.required(spec.atSlot(slot).getSchemaDefinitions())
             .getExecutionPayloadAndBlobsBundleSchema()
             .create(executionPayload, blobsBundle);
-    when(executionLayer.getCachedUnblindedPayload(slotAndBlockRoot))
+    when(executionLayer.getCachedUnblindedPayload(slot))
         .thenReturn(Optional.of(BuilderPayloadOrFallbackData.create(builderPayload)));
   }
 
   private void prepareCachedFallbackData(
-      final SlotAndBlockRoot slotAndBlockRoot,
-      final ExecutionPayload executionPayload,
-      final BlobsBundle blobsBundle) {
-    when(executionLayer.getCachedUnblindedPayload(slotAndBlockRoot))
+      final UInt64 slot, final ExecutionPayload executionPayload, final BlobsBundle blobsBundle) {
+    when(executionLayer.getCachedUnblindedPayload(slot))
         .thenReturn(
             Optional.of(
                 BuilderPayloadOrFallbackData.create(

ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerBlockProductionManagerImpl.java

@@ -22,7 +22,6 @@
 import tech.pegasys.teku.infrastructure.async.SafeFuture;
 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.execution.BuilderBidOrFallbackData;
 import tech.pegasys.teku.spec.datastructures.execution.BuilderPayloadOrFallbackData;
 import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadContext;
@@ -41,7 +40,7 @@ public class ExecutionLayerBlockProductionManagerImpl
   private final NavigableMap<UInt64, ExecutionPayloadResult> executionResultCache =
       new ConcurrentSkipListMap<>();
 
-  private final NavigableMap<SlotAndBlockRoot, BuilderPayloadOrFallbackData> builderResultCache =
+  private final NavigableMap<UInt64, BuilderPayloadOrFallbackData> builderResultCache =
       new ConcurrentSkipListMap<>();
 
   private final ExecutionLayerChannel executionLayerChannel;
@@ -56,13 +55,9 @@ public void onSlot(final UInt64 slot) {
     executionResultCache
         .headMap(slot.minusMinZero(EXECUTION_RESULT_CACHE_RETENTION_SLOTS), false)
         .clear();
-    final UInt64 slotMax = slot.minusMinZero(BUILDER_RESULT_CACHE_RETENTION_SLOTS);
-    builderResultCache.keySet().removeIf(key -> key.getSlot().isLessThan(slotMax));
-  }
-
-  @Override
-  public Optional<ExecutionPayloadResult> getCachedPayloadResult(final UInt64 slot) {
-    return Optional.ofNullable(executionResultCache.get(slot));
+    builderResultCache
+        .headMap(slot.minusMinZero(BUILDER_RESULT_CACHE_RETENTION_SLOTS), false)
+        .clear();
   }
 
   @Override
@@ -72,35 +67,42 @@ public ExecutionPayloadResult initiateBlockProduction(
       final boolean attemptBuilderFlow,
       final Optional<UInt64> requestedBuilderBoostFactor,
       final BlockProductionPerformance 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;
+    return executionResultCache.computeIfAbsent(
+        blockSlotState.getSlot(),
+        __ -> {
+          if (attemptBuilderFlow) {
+            return executeBuilderFlow(
+                context, blockSlotState, requestedBuilderBoostFactor, blockProductionPerformance);
+          } else {
+            return executeLocalFlow(context, blockSlotState, blockProductionPerformance);
+          }
+        });
+  }
+
+  @Override
+  public Optional<ExecutionPayloadResult> getCachedPayloadResult(final UInt64 slot) {
+    return Optional.ofNullable(executionResultCache.get(slot));
   }
 
   @Override
   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(
-                    signedBeaconBlock.getSlotAndBlockRoot(), builderPayloadOrFallbackData))
+                builderResultCache.put(slot, builderPayloadOrFallbackData))
         .alwaysRun(blockPublishingPerformance::builderGetPayload);
   }
 
   @Override
-  public Optional<BuilderPayloadOrFallbackData> getCachedUnblindedPayload(
-      final SlotAndBlockRoot slotAndBlockRoot) {
-    return Optional.ofNullable(builderResultCache.get(slotAndBlockRoot));
+  public Optional<BuilderPayloadOrFallbackData> getCachedUnblindedPayload(final UInt64 slot) {
+    return Optional.ofNullable(builderResultCache.get(slot));
   }
 
   private ExecutionPayloadResult executeLocalFlow(

ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/SignedBeaconBlock.java

@@ -137,11 +137,6 @@ public UInt64 getSlot() {
     return getMessage().getSlot();
   }
 
-  @Override
-  public SlotAndBlockRoot getSlotAndBlockRoot() {
-    return getMessage().getSlotAndBlockRoot();
-  }
-
   @Override
   public Bytes32 getParentRoot() {
     return getMessage().getParentRoot();

ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/SignedBlockContainer.java

@@ -40,10 +40,6 @@ default Bytes32 getRoot() {
     return getSignedBlock().getRoot();
   }
 
-  default SlotAndBlockRoot getSlotAndBlockRoot() {
-    return getSignedBlock().getSlotAndBlockRoot();
-  }
-
   default Optional<SszList<SszKZGProof>> getKzgProofs() {
     return Optional.empty();
   }

ethereum/spec/src/main/java/tech/pegasys/teku/spec/executionlayer/ExecutionLayerBlockProductionManager.java

@@ -19,7 +19,6 @@
 import tech.pegasys.teku.infrastructure.async.SafeFuture;
 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.execution.BuilderPayloadOrFallbackData;
 import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadContext;
 import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadResult;
@@ -34,11 +33,6 @@
 public interface ExecutionLayerBlockProductionManager {
   ExecutionLayerBlockProductionManager NOOP =
       new ExecutionLayerBlockProductionManager() {
-        @Override
-        public Optional<ExecutionPayloadResult> getCachedPayloadResult(final UInt64 slot) {
-          return Optional.empty();
-        }
-
         @Override
         public ExecutionPayloadResult initiateBlockProduction(
             final ExecutionPayloadContext context,
@@ -49,6 +43,11 @@ public ExecutionPayloadResult initiateBlockProduction(
           return null;
         }
 
+        @Override
+        public Optional<ExecutionPayloadResult> getCachedPayloadResult(final UInt64 slot) {
+          return Optional.empty();
+        }
+
         @Override
         public SafeFuture<BuilderPayloadOrFallbackData> getUnblindedPayload(
             final SignedBeaconBlock signedBeaconBlock,
@@ -57,8 +56,7 @@ public SafeFuture<BuilderPayloadOrFallbackData> getUnblindedPayload(
         }
 
         @Override
-        public Optional<BuilderPayloadOrFallbackData> getCachedUnblindedPayload(
-            final SlotAndBlockRoot slotAndBlockRoot) {
+        public Optional<BuilderPayloadOrFallbackData> getCachedUnblindedPayload(final UInt64 slot) {
           return Optional.empty();
         }
       };
@@ -95,6 +93,5 @@ SafeFuture<BuilderPayloadOrFallbackData> getUnblindedPayload(
    * Requires {@link #getUnblindedPayload(SignedBeaconBlock, BlockPublishingPerformance)} to have
    * been called first in order for a value to be present
    */
-  Optional<BuilderPayloadOrFallbackData> getCachedUnblindedPayload(
-      SlotAndBlockRoot slotAndBlockRoot);
+  Optional<BuilderPayloadOrFallbackData> getCachedUnblindedPayload(UInt64 slot);
 }