From 35610f6f16d91a439c71348a59aab3303c1e7441 Mon Sep 17 00:00:00 2001 From: Paul Harris Date: Tue, 21 Jan 2025 23:37:41 +1100 Subject: [PATCH 1/2] Added block production restrictions for electra (#9017) Signed-off-by: Paul Harris Co-authored-by: Enrico Del Fante --- .../BlockOperationSelectorFactory.java | 22 +++++++++++- .../BlockOperationSelectorFactoryTest.java | 34 +++++++++++++++++++ .../ethtests/finder/ReferenceTestFinder.java | 3 +- .../teku/spec/util/DataStructureUtil.java | 9 +++++ 4 files changed, 66 insertions(+), 2 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 7eff3b34f13..6b71266f7bf 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 @@ -57,6 +57,7 @@ import tech.pegasys.teku.spec.datastructures.operations.SignedVoluntaryExit; import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState; import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconStateCache; +import tech.pegasys.teku.spec.datastructures.state.versions.electra.PendingPartialWithdrawal; import tech.pegasys.teku.spec.datastructures.type.SszKZGCommitment; import tech.pegasys.teku.spec.datastructures.type.SszKZGProof; import tech.pegasys.teku.spec.executionlayer.ExecutionLayerBlockProductionManager; @@ -146,7 +147,7 @@ public Function> createSelector( final SszList voluntaryExits = voluntaryExitPool.getItemsForBlock( blockSlotState, - exit -> !exitedValidators.contains(exit.getMessage().getValidatorIndex()), + exit -> voluntaryExitPredicate(blockSlotState, exitedValidators, exit), exit -> exitedValidators.add(exit.getMessage().getValidatorIndex())); bodyBuilder @@ -204,6 +205,25 @@ public Function> createSelector( }; } + private boolean voluntaryExitPredicate( + final BeaconState blockSlotState, + final Set exitedValidators, + final SignedVoluntaryExit exit) { + final UInt64 validatorIndex = exit.getMessage().getValidatorIndex(); + if (exitedValidators.contains(validatorIndex)) { + return false; + } + // if there is a pending withdrawal, the exit is not valid for inclusion in a block. + return blockSlotState + .toVersionElectra() + .map( + beaconStateElectra -> + beaconStateElectra.getPendingPartialWithdrawals().stream() + .map(PendingPartialWithdrawal::getValidatorIndex) + .noneMatch(index -> index == validatorIndex.intValue())) + .orElse(true); + } + private SafeFuture setExecutionData( final Optional executionPayloadContext, final BeaconBlockBodyBuilder bodyBuilder, 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 35c58e2a331..331e4ac3832 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 @@ -78,6 +78,7 @@ import tech.pegasys.teku.spec.datastructures.operations.versions.altair.SignedContributionAndProof; import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState; import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconStateCache; +import tech.pegasys.teku.spec.datastructures.state.beaconstate.versions.electra.MutableBeaconStateElectra; import tech.pegasys.teku.spec.datastructures.type.SszKZGCommitment; import tech.pegasys.teku.spec.datastructures.type.SszKZGProof; import tech.pegasys.teku.spec.executionlayer.ExecutionLayerBlockProductionManager; @@ -264,6 +265,39 @@ void shouldNotSelectOperationsWhenNoneAreAvailable() { assertThat(bodyBuilder.blsToExecutionChanges).isEmpty(); } + @Test + void shouldNotSelectVoluntaryExitWhenValidatorHasPendingWithdrawal() { + final UInt64 slot = UInt64.ONE; + final MutableBeaconStateElectra blockSlotState = + dataStructureUtil.randomBeaconState(slot).toVersionElectra().get().createWritableCopy(); + blockSlotState + .getPendingPartialWithdrawals() + .append(dataStructureUtil.randomPendingPartialWithdrawal(1)); + final SignedVoluntaryExit voluntaryExit = + dataStructureUtil.randomSignedVoluntaryExit(UInt64.valueOf(1)); + final ExecutionPayload randomExecutionPayload = dataStructureUtil.randomExecutionPayload(); + final UInt256 blockExecutionValue = dataStructureUtil.randomUInt256(); + + addToPool(voluntaryExitPool, voluntaryExit); + prepareBlockProductionWithPayload( + randomExecutionPayload, + executionPayloadContext, + blockSlotState, + Optional.of(blockExecutionValue)); + + safeJoin( + factory + .createSelector( + parentRoot, + blockSlotState, + randaoReveal, + Optional.of(defaultGraffiti), + Optional.empty(), + BlockProductionPerformance.NOOP) + .apply(bodyBuilder)); + assertThat(bodyBuilder.voluntaryExits).isEmpty(); + } + @Test void shouldIncludeValidOperations() { final UInt64 slot = UInt64.valueOf(2); diff --git a/eth-tests/src/main/java/tech/pegasys/teku/ethtests/finder/ReferenceTestFinder.java b/eth-tests/src/main/java/tech/pegasys/teku/ethtests/finder/ReferenceTestFinder.java index 4dbcf5ac10a..d58be7186fa 100644 --- a/eth-tests/src/main/java/tech/pegasys/teku/ethtests/finder/ReferenceTestFinder.java +++ b/eth-tests/src/main/java/tech/pegasys/teku/ethtests/finder/ReferenceTestFinder.java @@ -35,7 +35,8 @@ public class ReferenceTestFinder { TestFork.ALTAIR, TestFork.BELLATRIX, TestFork.CAPELLA, - TestFork.DENEB); // TODO: Add Electra fork tests back + TestFork.DENEB, + TestFork.ELECTRA); @MustBeClosed public static Stream findReferenceTests() throws IOException { diff --git a/ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java b/ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java index 7e312c58cf7..13349943cf1 100644 --- a/ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java +++ b/ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java @@ -2641,6 +2641,15 @@ public PendingPartialWithdrawal randomPendingPartialWithdrawal() { SszUInt64.of(randomUInt64())); } + public PendingPartialWithdrawal randomPendingPartialWithdrawal(final long validatorIndex) { + return getElectraSchemaDefinitions(randomSlot()) + .getPendingPartialWithdrawalSchema() + .create( + SszUInt64.of(UInt64.valueOf(validatorIndex)), + SszUInt64.of(randomUInt64()), + SszUInt64.of(randomUInt64())); + } + public UInt64 randomBlobSidecarIndex() { return randomUInt64( spec.forMilestone(spec.getForkSchedule().getHighestSupportedMilestone()) From 1e371eec40ca538867cd8be39bd49f832ae25172 Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Tue, 21 Jan 2025 14:11:12 +0000 Subject: [PATCH 2/2] Handle overflows when validating BlobSidecarsByRange requests (#9019) --- .../BlobSidecarsByRangeMessageHandler.java | 3 +- ...BlobSidecarsByRangeMessageHandlerTest.java | 34 +++++++++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BlobSidecarsByRangeMessageHandler.java b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BlobSidecarsByRangeMessageHandler.java index 55ba2e5a484..56623d27d94 100644 --- a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BlobSidecarsByRangeMessageHandler.java +++ b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BlobSidecarsByRangeMessageHandler.java @@ -93,7 +93,8 @@ public Optional validateRequest( final long requestedCount = calculateRequestedCount(request, maxBlobsPerBlock); - if (requestedCount > maxRequestBlobSidecars) { + // handle overflows + if (requestedCount < 0 || requestedCount > maxRequestBlobSidecars) { requestCounter.labels("count_too_big").inc(); return Optional.of( new RpcException( diff --git a/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BlobSidecarsByRangeMessageHandlerTest.java b/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BlobSidecarsByRangeMessageHandlerTest.java index be33be74933..c5e8c6305f0 100644 --- a/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BlobSidecarsByRangeMessageHandlerTest.java +++ b/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BlobSidecarsByRangeMessageHandlerTest.java @@ -166,13 +166,43 @@ public void validateRequest_shouldRejectRequestWhenCountIsTooBig() { "Only a maximum of %s blob sidecars can be requested per request", maxRequestBlobSidecars))); - final long rateLimitedCount = + final long countTooBigCount = metricsSystem.getCounterValue( TekuMetricCategory.NETWORK, "rpc_blob_sidecars_by_range_requests_total", "count_too_big"); - assertThat(rateLimitedCount).isOne(); + assertThat(countTooBigCount).isOne(); + } + + @TestTemplate + public void validateRequest_shouldRejectRequestForVeryLargeValue() { + final UInt64 maxRequestBlobSidecars = + UInt64.valueOf( + SpecConfigDeneb.required(spec.forMilestone(specMilestone).getConfig()) + .getMaxRequestBlobSidecars()); + final BlobSidecarsByRangeRequestMessage request = + new BlobSidecarsByRangeRequestMessage( + // bypass ArithmeticException when calculating maxSlot + startSlot, UInt64.MAX_VALUE.minus(startSlot), maxBlobsPerBlock); + + final Optional result = handler.validateRequest(protocolId, request); + + assertThat(result) + .hasValue( + new RpcException( + INVALID_REQUEST_CODE, + String.format( + "Only a maximum of %s blob sidecars can be requested per request", + maxRequestBlobSidecars))); + + final long countTooBigCount = + metricsSystem.getCounterValue( + TekuMetricCategory.NETWORK, + "rpc_blob_sidecars_by_range_requests_total", + "count_too_big"); + + assertThat(countTooBigCount).isOne(); } @TestTemplate