Change the builderResultCache of ExecutionLayerBlockProductionManagerImpl (#8766)

Change the builderResultCache of ExecutionLayerBlockProductionManagerImpl keys from slot to SlotAndBlockRoot. This allows the cache to return proposals based on slot and block root, which is required if there's multiple operators requesting different beacon block proposals.

fixes #8625

Author: David Ryan

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(slot)
+                .getCachedUnblindedPayload(block.getSlotAndBlockRoot())
                 .orElseThrow(
                     () ->
                         new IllegalStateException(

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

@@ -374,7 +374,8 @@ protected BlockAndBlobSidecars createBlockAndBlobSidecars(
     }
 
     // simulate caching of the builder payload
-    when(executionLayer.getCachedUnblindedPayload(signedBlockContainer.getSlot()))
+    when(executionLayer.getCachedUnblindedPayload(
+            signedBlockContainer.getSignedBlock().getSlotAndBlockRoot()))
         .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.getSlot());
+    verify(executionLayer).getCachedUnblindedPayload(block.getSlotAndBlockRoot());
 
     final SszList<SszKZGCommitment> expectedCommitments =
         block.getSignedBlock().getMessage().getBody().getOptionalBlobKzgCommitments().orElseThrow();

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

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

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

@@ -22,6 +22,7 @@
 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;
@@ -40,7 +41,7 @@ public class ExecutionLayerBlockProductionManagerImpl
   private final NavigableMap<UInt64, ExecutionPayloadResult> executionResultCache =
       new ConcurrentSkipListMap<>();
 
-  private final NavigableMap<UInt64, BuilderPayloadOrFallbackData> builderResultCache =
+  private final NavigableMap<SlotAndBlockRoot, BuilderPayloadOrFallbackData> builderResultCache =
       new ConcurrentSkipListMap<>();
 
   private final ExecutionLayerChannel executionLayerChannel;
@@ -55,9 +56,8 @@ public void onSlot(final UInt64 slot) {
     executionResultCache
         .headMap(slot.minusMinZero(EXECUTION_RESULT_CACHE_RETENTION_SLOTS), false)
         .clear();
-    builderResultCache
-        .headMap(slot.minusMinZero(BUILDER_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
@@ -92,13 +92,15 @@ public SafeFuture<BuilderPayloadOrFallbackData> getUnblindedPayload(
         .builderGetPayload(signedBeaconBlock, this::getCachedPayloadResult)
         .thenPeek(
             builderPayloadOrFallbackData ->
-                builderResultCache.put(signedBeaconBlock.getSlot(), builderPayloadOrFallbackData))
+                builderResultCache.put(
+                    signedBeaconBlock.getSlotAndBlockRoot(), builderPayloadOrFallbackData))
         .alwaysRun(blockPublishingPerformance::builderGetPayload);
   }
 
   @Override
-  public Optional<BuilderPayloadOrFallbackData> getCachedUnblindedPayload(final UInt64 slot) {
-    return Optional.ofNullable(builderResultCache.get(slot));
+  public Optional<BuilderPayloadOrFallbackData> getCachedUnblindedPayload(
+      final SlotAndBlockRoot slotAndBlockRoot) {
+    return Optional.ofNullable(builderResultCache.get(slotAndBlockRoot));
   }
 
   private ExecutionPayloadResult executeLocalFlow(

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

@@ -137,6 +137,11 @@ 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,6 +40,10 @@ 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,6 +19,7 @@
 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;
@@ -56,7 +57,8 @@ public SafeFuture<BuilderPayloadOrFallbackData> getUnblindedPayload(
         }
 
         @Override
-        public Optional<BuilderPayloadOrFallbackData> getCachedUnblindedPayload(final UInt64 slot) {
+        public Optional<BuilderPayloadOrFallbackData> getCachedUnblindedPayload(
+            final SlotAndBlockRoot slotAndBlockRoot) {
           return Optional.empty();
         }
       };
@@ -93,5 +95,6 @@ SafeFuture<BuilderPayloadOrFallbackData> getUnblindedPayload(
    * Requires {@link #getUnblindedPayload(SignedBeaconBlock, BlockPublishingPerformance)} to have
    * been called first in order for a value to be present
    */
-  Optional<BuilderPayloadOrFallbackData> getCachedUnblindedPayload(UInt64 slot);
+  Optional<BuilderPayloadOrFallbackData> getCachedUnblindedPayload(
+      SlotAndBlockRoot slotAndBlockRoot);
 }