From c763d1cc46ebdceb8a2ae508d612f7901e2bf72e Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Wed, 23 Oct 2024 14:53:29 +0800 Subject: [PATCH] Caching changes in ExecutionLayerBlockProductionManager --- .../BlockOperationSelectorFactory.java | 2 +- .../coordinator/AbstractBlockFactoryTest.java | 3 +- .../coordinator/BlockFactoryDenebTest.java | 2 +- .../BlockOperationSelectorFactoryTest.java | 28 ++++------- ...cutionLayerBlockProductionManagerImpl.java | 50 ++++++++++--------- .../blocks/SignedBeaconBlock.java | 5 -- .../blocks/SignedBlockContainer.java | 4 -- .../ExecutionLayerBlockProductionManager.java | 17 +++---- 8 files changed, 46 insertions(+), 65 deletions(-) diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactory.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactory.java index 19121d652c8..20357e8c7c3 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactory.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactory.java @@ -465,7 +465,7 @@ public Function> 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( diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/AbstractBlockFactoryTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/AbstractBlockFactoryTest.java index e17fd2ec7a6..e0197e631cd 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/AbstractBlockFactoryTest.java +++ b/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 blobSidecars = diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDenebTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDenebTest.java index 3ce46e1fcc5..67103b9e60e 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDenebTest.java +++ b/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 blobSidecars = blockAndBlobSidecars.blobSidecars(); - verify(executionLayer).getCachedUnblindedPayload(block.getSlotAndBlockRoot()); + verify(executionLayer).getCachedUnblindedPayload(block.getSlot()); final SszList expectedCommitments = block.getSignedBlock().getMessage().getBody().getOptionalBlobKzgCommitments().orElseThrow(); diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java index a656612824d..1370795c08e 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java +++ b/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 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( diff --git a/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerBlockProductionManagerImpl.java b/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerBlockProductionManagerImpl.java index dd4e00c644c..1cb3db06408 100644 --- a/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerBlockProductionManagerImpl.java +++ b/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 executionResultCache = new ConcurrentSkipListMap<>(); - private final NavigableMap builderResultCache = + private final NavigableMap 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 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 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 getCachedPayloadResult(final UInt64 slot) { + return Optional.ofNullable(executionResultCache.get(slot)); } @Override public SafeFuture 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 getCachedUnblindedPayload( - final SlotAndBlockRoot slotAndBlockRoot) { - return Optional.ofNullable(builderResultCache.get(slotAndBlockRoot)); + public Optional getCachedUnblindedPayload(final UInt64 slot) { + return Optional.ofNullable(builderResultCache.get(slot)); } private ExecutionPayloadResult executeLocalFlow( diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/SignedBeaconBlock.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/SignedBeaconBlock.java index 1db4c22635b..7c814014906 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/SignedBeaconBlock.java +++ b/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(); diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/SignedBlockContainer.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/SignedBlockContainer.java index 3cadfd77da6..fa8dccf4457 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blocks/SignedBlockContainer.java +++ b/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> getKzgProofs() { return Optional.empty(); } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/executionlayer/ExecutionLayerBlockProductionManager.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/executionlayer/ExecutionLayerBlockProductionManager.java index 950755e1e23..435be8b119f 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/executionlayer/ExecutionLayerBlockProductionManager.java +++ b/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 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 getCachedPayloadResult(final UInt64 slot) { + return Optional.empty(); + } + @Override public SafeFuture getUnblindedPayload( final SignedBeaconBlock signedBeaconBlock, @@ -57,8 +56,7 @@ public SafeFuture getUnblindedPayload( } @Override - public Optional getCachedUnblindedPayload( - final SlotAndBlockRoot slotAndBlockRoot) { + public Optional getCachedUnblindedPayload(final UInt64 slot) { return Optional.empty(); } }; @@ -95,6 +93,5 @@ SafeFuture getUnblindedPayload( * Requires {@link #getUnblindedPayload(SignedBeaconBlock, BlockPublishingPerformance)} to have * been called first in order for a value to be present */ - Optional getCachedUnblindedPayload( - SlotAndBlockRoot slotAndBlockRoot); + Optional getCachedUnblindedPayload(UInt64 slot); }