From 14aa66e3542572eed88ec77c504ac71e7b425e07 Mon Sep 17 00:00:00 2001 From: David Ryan Date: Tue, 22 Oct 2024 15:46:56 +1100 Subject: [PATCH 1/2] 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. --- .../BlockOperationSelectorFactory.java | 2 +- .../coordinator/AbstractBlockFactoryTest.java | 3 +- .../coordinator/BlockFactoryDenebTest.java | 2 +- .../BlockOperationSelectorFactoryTest.java | 28 ++++++++++++------- ...cutionLayerBlockProductionManagerImpl.java | 18 +++++++----- .../ExecutionLayerBlockProductionManager.java | 7 +++-- 6 files changed, 38 insertions(+), 22 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 20357e8c7c3..19121d652c8 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(slot) + .getCachedUnblindedPayload(block.getSlotAndBlockRoot()) .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 e0197e631cd..e17fd2ec7a6 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,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 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 67103b9e60e..4142114e354 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.getSlot()); + verify(executionLayer).getCachedUnblindedPayload(block.getSignedBlock().getSlotAndBlockRoot()); 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 1370795c08e..a656612824d 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,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 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( 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 2ae5804c7c9..4766b33f2cb 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 @@ -16,12 +16,14 @@ import java.util.NavigableMap; import java.util.Optional; import java.util.concurrent.ConcurrentSkipListMap; +import org.apache.tuweni.bytes.Bytes32; import tech.pegasys.teku.ethereum.events.SlotEventsChannel; import tech.pegasys.teku.ethereum.performance.trackers.BlockProductionPerformance; import tech.pegasys.teku.ethereum.performance.trackers.BlockPublishingPerformance; 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 +42,7 @@ public class ExecutionLayerBlockProductionManagerImpl private final NavigableMap executionResultCache = new ConcurrentSkipListMap<>(); - private final NavigableMap builderResultCache = + private final NavigableMap builderResultCache = new ConcurrentSkipListMap<>(); private final ExecutionLayerChannel executionLayerChannel; @@ -55,9 +57,9 @@ 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(); + SlotAndBlockRoot toSlotAndBlockRoot = + new SlotAndBlockRoot(slot.minusMinZero(BUILDER_RESULT_CACHE_RETENTION_SLOTS), Bytes32.ZERO); + builderResultCache.headMap(toSlotAndBlockRoot, false).clear(); } @Override @@ -92,13 +94,15 @@ public SafeFuture getUnblindedPayload( .builderGetPayload(signedBeaconBlock, this::getCachedPayloadResult) .thenPeek( builderPayloadOrFallbackData -> - builderResultCache.put(signedBeaconBlock.getSlot(), builderPayloadOrFallbackData)) + builderResultCache.put( + signedBeaconBlock.getSlotAndBlockRoot(), builderPayloadOrFallbackData)) .alwaysRun(blockPublishingPerformance::builderGetPayload); } @Override - public Optional getCachedUnblindedPayload(final UInt64 slot) { - return Optional.ofNullable(builderResultCache.get(slot)); + public Optional getCachedUnblindedPayload( + final SlotAndBlockRoot slotAndBlockRoot) { + return Optional.ofNullable(builderResultCache.get(slotAndBlockRoot)); } private ExecutionPayloadResult executeLocalFlow( 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 0eae9cec5af..950755e1e23 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,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 getUnblindedPayload( } @Override - public Optional getCachedUnblindedPayload(final UInt64 slot) { + public Optional getCachedUnblindedPayload( + final SlotAndBlockRoot slotAndBlockRoot) { return Optional.empty(); } }; @@ -93,5 +95,6 @@ SafeFuture getUnblindedPayload( * Requires {@link #getUnblindedPayload(SignedBeaconBlock, BlockPublishingPerformance)} to have * been called first in order for a value to be present */ - Optional getCachedUnblindedPayload(UInt64 slot); + Optional getCachedUnblindedPayload( + SlotAndBlockRoot slotAndBlockRoot); } From c189cf27294925c6cf23dee697fddd9b8df1eb0e Mon Sep 17 00:00:00 2001 From: David Ryan Date: Wed, 23 Oct 2024 09:22:02 +1100 Subject: [PATCH 2/2] A few fixes based on feedback. Added getSlotAndBlockRoot default to SignedBlockContainer which required updating the SignedBeaconBlock implementation. --- .../teku/validator/coordinator/BlockFactoryDenebTest.java | 2 +- .../ExecutionLayerBlockProductionManagerImpl.java | 6 ++---- .../teku/spec/datastructures/blocks/SignedBeaconBlock.java | 5 +++++ .../spec/datastructures/blocks/SignedBlockContainer.java | 4 ++++ 4 files changed, 12 insertions(+), 5 deletions(-) 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 4142114e354..3ce46e1fcc5 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.getSignedBlock().getSlotAndBlockRoot()); + verify(executionLayer).getCachedUnblindedPayload(block.getSlotAndBlockRoot()); final SszList expectedCommitments = block.getSignedBlock().getMessage().getBody().getOptionalBlobKzgCommitments().orElseThrow(); 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 4766b33f2cb..dd4e00c644c 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 @@ -16,7 +16,6 @@ import java.util.NavigableMap; import java.util.Optional; import java.util.concurrent.ConcurrentSkipListMap; -import org.apache.tuweni.bytes.Bytes32; import tech.pegasys.teku.ethereum.events.SlotEventsChannel; import tech.pegasys.teku.ethereum.performance.trackers.BlockProductionPerformance; import tech.pegasys.teku.ethereum.performance.trackers.BlockPublishingPerformance; @@ -57,9 +56,8 @@ public void onSlot(final UInt64 slot) { executionResultCache .headMap(slot.minusMinZero(EXECUTION_RESULT_CACHE_RETENTION_SLOTS), false) .clear(); - SlotAndBlockRoot toSlotAndBlockRoot = - new SlotAndBlockRoot(slot.minusMinZero(BUILDER_RESULT_CACHE_RETENTION_SLOTS), Bytes32.ZERO); - builderResultCache.headMap(toSlotAndBlockRoot, false).clear(); + final UInt64 slotMax = slot.minusMinZero(BUILDER_RESULT_CACHE_RETENTION_SLOTS); + builderResultCache.keySet().removeIf(key -> key.getSlot().isLessThan(slotMax)); } @Override 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 7c814014906..1db4c22635b 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,6 +137,11 @@ 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 fa8dccf4457..3cadfd77da6 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,6 +40,10 @@ default Bytes32 getRoot() { return getSignedBlock().getRoot(); } + default SlotAndBlockRoot getSlotAndBlockRoot() { + return getSignedBlock().getSlotAndBlockRoot(); + } + default Optional> getKzgProofs() { return Optional.empty(); }