Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove --xblock-v3-enabled and useBlindedBlock variable #8511

Merged
merged 23 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ SafeFuture<BlockContainerAndMetaData> createUnsignedBlock(
UInt64 proposalSlot,
BLSSignature randaoReveal,
Optional<Bytes32> optionalGraffiti,
Optional<Boolean> requestedBlinded,
Optional<UInt64> requestedBuilderBoostFactor,
BlockProductionPerformance blockProductionPerformance);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,13 @@ public SafeFuture<BlockContainerAndMetaData> createUnsignedBlock(
final UInt64 proposalSlot,
final BLSSignature randaoReveal,
final Optional<Bytes32> optionalGraffiti,
final Optional<Boolean> requestedBlinded,
final Optional<UInt64> requestedBuilderBoostFactor,
final BlockProductionPerformance blockProductionPerformance) {
return super.createUnsignedBlock(
blockSlotState,
proposalSlot,
randaoReveal,
optionalGraffiti,
requestedBlinded,
requestedBuilderBoostFactor,
blockProductionPerformance)
.thenCompose(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ public SafeFuture<BlockContainerAndMetaData> createUnsignedBlock(
final UInt64 proposalSlot,
final BLSSignature randaoReveal,
final Optional<Bytes32> optionalGraffiti,
final Optional<Boolean> requestedBlinded,
final Optional<UInt64> requestedBuilderBoostFactor,
final BlockProductionPerformance blockProductionPerformance) {
checkArgument(
Expand All @@ -76,7 +75,6 @@ public SafeFuture<BlockContainerAndMetaData> createUnsignedBlock(
blockSlotState,
randaoReveal,
optionalGraffiti,
requestedBlinded,
requestedBuilderBoostFactor,
blockProductionPerformance),
blockProductionPerformance)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ public Function<BeaconBlockBodyBuilder, SafeFuture<Void>> createSelector(
final BeaconState blockSlotState,
final BLSSignature randaoReveal,
final Optional<Bytes32> optionalGraffiti,
final Optional<Boolean> requestedBlinded,
final Optional<UInt64> requestedBuilderBoostFactor,
final BlockProductionPerformance blockProductionPerformance) {

Expand Down Expand Up @@ -190,7 +189,6 @@ public Function<BeaconBlockBodyBuilder, SafeFuture<Void>> createSelector(
setExecutionData(
executionPayloadContext,
bodyBuilder,
requestedBlinded,
requestedBuilderBoostFactor,
SchemaDefinitionsBellatrix.required(schemaDefinitions),
blockSlotState,
Expand All @@ -208,7 +206,6 @@ public Function<BeaconBlockBodyBuilder, SafeFuture<Void>> createSelector(
private SafeFuture<Void> setExecutionData(
final Optional<ExecutionPayloadContext> executionPayloadContext,
final BeaconBlockBodyBuilder bodyBuilder,
final Optional<Boolean> requestedBlinded,
final Optional<UInt64> requestedBuilderBoostFactor,
final SchemaDefinitionsBellatrix schemaDefinitions,
final BeaconState blockSlotState,
Expand All @@ -224,11 +221,9 @@ private SafeFuture<Void> setExecutionData(
// if requestedBlinded has been specified, we strictly follow it, otherwise we should run
gfukushima marked this conversation as resolved.
Show resolved Hide resolved
// Builder flow (blinded) only if we have a validator registration
final boolean shouldTryBuilderFlow =
requestedBlinded.orElseGet(
() ->
executionPayloadContext
.map(ExecutionPayloadContext::isValidatorRegistrationPresent)
.orElse(false));
.orElse(false);

// pre-Merge Execution Payload / Execution Payload Header
if (executionPayloadContext.isEmpty()) {
Expand All @@ -252,7 +247,7 @@ private SafeFuture<Void> setExecutionData(
return SafeFuture.allOf(
cacheExecutionPayloadValue(executionPayloadResult, blockSlotState),
setPayloadOrPayloadHeader(
bodyBuilder, schemaDefinitions, requestedBlinded, executionPayloadResult),
bodyBuilder, schemaDefinitions, executionPayloadResult),
setKzgCommitments(bodyBuilder, schemaDefinitions, executionPayloadResult));
}

Expand All @@ -269,7 +264,6 @@ private SafeFuture<Void> cacheExecutionPayloadValue(
private SafeFuture<Void> setPayloadOrPayloadHeader(
final BeaconBlockBodyBuilder bodyBuilder,
final SchemaDefinitionsBellatrix schemaDefinitions,
final Optional<Boolean> requestedBlinded,
final ExecutionPayloadResult executionPayloadResult) {

if (executionPayloadResult.isFromLocalFlow()) {
Expand All @@ -287,8 +281,7 @@ private SafeFuture<Void> setPayloadOrPayloadHeader(
builderBidOrFallbackData -> {
// we should try to return unblinded content only if no explicit "blindness" has been
// requested and builder flow fallbacks to local
if (requestedBlinded.isEmpty()
&& builderBidOrFallbackData.getFallbackData().isPresent()) {
if (builderBidOrFallbackData.getFallbackData().isPresent()) {
Copy link
Contributor

@StefanBratanov StefanBratanov Aug 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole if statement should be simplified. After this change, we have only one condition, if fallback data is present, we return unblinded executionPayload, otherwise we have the builderBid and return the blinded header.

Essentially, we can skip this, since it will never happen:

final ExecutionPayload executionPayload =builderBidOrFallbackData
    .getFallbackDataRequired()
    .getExecutionPayload();
 return schemaDefinitions
     .getExecutionPayloadHeaderSchema()
     .createFromExecutionPayload(executionPayload);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well that's interesting, I understand the logic behind this clean up but wouldn't that be somehow a weird enforcement we would do? Every CL must have a EL in order to be fully functional in the network as a proposer, no? If so the first condition will always be true and even if you configure a builderBid we will default to use the fallback always isn't that so?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the way it works is that either the builder bid is present or the fallback is present. The two won't be present at the same time.

bodyBuilder.executionPayload(
builderBidOrFallbackData.getFallbackDataRequired().getExecutionPayload());
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ public SafeFuture<BlockContainerAndMetaData> createUnsignedBlock(
final UInt64 proposalSlot,
final BLSSignature randaoReveal,
final Optional<Bytes32> optionalGraffiti,
final Optional<Boolean> requestedBlinded,
final Optional<UInt64> requestedBuilderBoostFactor,
final BlockProductionPerformance blockProductionPerformance) {
final SpecMilestone milestone = getMilestone(proposalSlot);
Expand All @@ -78,7 +77,6 @@ public SafeFuture<BlockContainerAndMetaData> createUnsignedBlock(
proposalSlot,
randaoReveal,
optionalGraffiti,
requestedBlinded,
requestedBuilderBoostFactor,
blockProductionPerformance);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,6 @@ public SafeFuture<Optional<BlockContainerAndMetaData>> createUnsignedBlock(
final UInt64 slot,
final BLSSignature randaoReveal,
final Optional<Bytes32> graffiti,
final Optional<Boolean> requestedBlinded,
final Optional<UInt64> requestedBuilderBoostFactor) {
LOG.info("Creating unsigned block for slot {}", slot);
performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(slot));
Expand All @@ -357,7 +356,6 @@ public SafeFuture<Optional<BlockContainerAndMetaData>> createUnsignedBlock(
slot,
randaoReveal,
graffiti,
requestedBlinded,
requestedBuilderBoostFactor,
blockSlotState,
blockProductionPerformance))
Expand All @@ -368,7 +366,6 @@ private SafeFuture<Optional<BlockContainerAndMetaData>> createBlock(
final UInt64 slot,
final BLSSignature randaoReveal,
final Optional<Bytes32> graffiti,
final Optional<Boolean> requestedBlinded,
final Optional<UInt64> requestedBuilderBoostFactor,
final Optional<BeaconState> maybeBlockSlotState,
final BlockProductionPerformance blockProductionPerformance) {
Expand All @@ -390,7 +387,6 @@ private SafeFuture<Optional<BlockContainerAndMetaData>> createBlock(
slot,
randaoReveal,
graffiti,
requestedBlinded,
requestedBuilderBoostFactor,
blockProductionPerformance)
.thenApply(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ protected BlockContainerAndMetaData assertBlockCreated(
newSlot,
randaoReveal,
Optional.empty(),
Optional.of(blinded),
Optional.empty(),
BlockProductionPerformance.NOOP));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,6 @@ void shouldNotSelectOperationsWhenNoneAreAvailable() {
dataStructureUtil.randomSignature(),
Optional.empty(),
Optional.empty(),
Optional.empty(),
BlockProductionPerformance.NOOP)
.apply(bodyBuilder));

Expand Down Expand Up @@ -303,7 +302,6 @@ void shouldIncludeValidOperations() {
randaoReveal,
Optional.of(defaultGraffiti),
Optional.empty(),
Optional.empty(),
BlockProductionPerformance.NOOP)
.apply(bodyBuilder));

Expand Down Expand Up @@ -391,7 +389,6 @@ void shouldNotIncludeInvalidOperations() {
randaoReveal,
Optional.of(defaultGraffiti),
Optional.empty(),
Optional.empty(),
BlockProductionPerformance.NOOP)
.apply(bodyBuilder));

Expand Down Expand Up @@ -422,7 +419,6 @@ void shouldIncludeDefaultExecutionPayload() {
blockSlotState,
dataStructureUtil.randomSignature(),
Optional.empty(),
Optional.of(false),
Optional.empty(),
BlockProductionPerformance.NOOP)
.apply(bodyBuilder));
Expand All @@ -443,7 +439,6 @@ void shouldIncludeExecutionPayloadHeaderOfDefaultPayload() {
blockSlotState,
dataStructureUtil.randomSignature(),
Optional.empty(),
Optional.of(true),
Optional.empty(),
BlockProductionPerformance.NOOP)
.apply(bodyBuilder));
Expand Down Expand Up @@ -471,7 +466,6 @@ void shouldIncludeNonDefaultExecutionPayload() {
blockSlotState,
dataStructureUtil.randomSignature(),
Optional.empty(),
Optional.of(false),
Optional.empty(),
BlockProductionPerformance.NOOP)
.apply(bodyBuilder));
Expand Down Expand Up @@ -503,7 +497,6 @@ void shouldIncludeExecutionPayloadHeaderIfBlindedBlockRequested() {
blockSlotState,
dataStructureUtil.randomSignature(),
Optional.empty(),
Optional.of(true),
Optional.empty(),
BlockProductionPerformance.NOOP)
.apply(bodyBuilder));
Expand Down Expand Up @@ -534,7 +527,6 @@ void shouldIncludeExecutionPayloadIfUnblindedBlockRequested() {
blockSlotState,
dataStructureUtil.randomSignature(),
Optional.empty(),
Optional.of(false),
Optional.empty(),
BlockProductionPerformance.NOOP)
.apply(bodyBuilder));
Expand Down Expand Up @@ -566,7 +558,6 @@ void shouldIncludeExecutionPayloadIfRequestedBlindedIsEmpty() {
dataStructureUtil.randomSignature(),
Optional.empty(),
Optional.empty(),
Optional.empty(),
BlockProductionPerformance.NOOP)
.apply(bodyBuilder));

Expand Down Expand Up @@ -598,7 +589,6 @@ void shouldIncludeExecutionPayloadIfRequestedBlindedIsEmptyAndBuilderFlowFallsBa
dataStructureUtil.randomSignature(),
Optional.empty(),
Optional.empty(),
Optional.empty(),
BlockProductionPerformance.NOOP)
.apply(bodyBuilder));

Expand Down Expand Up @@ -640,7 +630,6 @@ void shouldIncludeExecutionPayloadIfRequestedBlindedIsEmptyAndBuilderFlowFallsBa
dataStructureUtil.randomSignature(),
Optional.empty(),
Optional.empty(),
Optional.empty(),
BlockProductionPerformance.NOOP)
.apply(bodyBuilder));

Expand Down Expand Up @@ -704,7 +693,6 @@ void shouldIncludeKzgCommitmentsInBlock() {
blockSlotState,
dataStructureUtil.randomSignature(),
Optional.empty(),
Optional.of(false),
Optional.empty(),
BlockProductionPerformance.NOOP)
.apply(bodyBuilder));
Expand Down Expand Up @@ -745,8 +733,7 @@ void shouldIncludeKzgCommitmentsInBlindedBlock() {
blockSlotState,
dataStructureUtil.randomSignature(),
Optional.empty(),
Optional.of(true),
Optional.empty(),
Optional.of(UInt64.ONE),
BlockProductionPerformance.NOOP)
.apply(bodyBuilder));

Expand Down Expand Up @@ -979,7 +966,6 @@ void shouldThrowWhenExecutionPayloadContextNotProvided() {
blockSlotState,
dataStructureUtil.randomSignature(),
Optional.empty(),
Optional.of(false),
Optional.empty(),
BlockProductionPerformance.NOOP)
.apply(bodyBuilder))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,6 @@ public void createUnsignedBlock_shouldFailWhenNodeIsSyncing() {
ONE,
dataStructureUtil.randomSignature(),
Optional.empty(),
Optional.of(false),
Optional.of(ONE));

assertThat(result).isCompletedExceptionally();
Expand All @@ -521,7 +520,6 @@ public void createUnsignedBlock_shouldFailWhenParentBlockIsOptimistic() {
newSlot,
dataStructureUtil.randomSignature(),
Optional.empty(),
Optional.of(false),
Optional.of(ONE));

assertThat(result).isCompletedExceptionally();
Expand All @@ -544,7 +542,6 @@ public void createUnsignedBlock_shouldCreateBlock() {
newSlot,
randaoReveal,
Optional.empty(),
Optional.of(false),
Optional.of(ONE),
BlockProductionPerformance.NOOP))
.thenReturn(SafeFuture.completedFuture(blockContainerAndMetaData));
Expand All @@ -554,15 +551,14 @@ public void createUnsignedBlock_shouldCreateBlock() {
// we still want to check that all parameters are passed down the line to the block factory
final SafeFuture<Optional<BlockContainerAndMetaData>> result =
validatorApiHandler.createUnsignedBlock(
newSlot, randaoReveal, Optional.empty(), Optional.of(false), Optional.of(ONE));
newSlot, randaoReveal, Optional.empty(), Optional.of(ONE));

verify(blockFactory)
.createUnsignedBlock(
blockSlotState,
newSlot,
randaoReveal,
Optional.empty(),
Optional.of(false),
Optional.of(ONE),
BlockProductionPerformance.NOOP);
assertThat(result).isCompletedWithValue(Optional.of(blockContainerAndMetaData));
Expand Down Expand Up @@ -915,14 +911,13 @@ public void sendSignedBlock_shouldConvertKnownBlockResult() {
newSlot,
randaoReveal,
Optional.empty(),
Optional.of(false),
Optional.of(ONE),
BlockProductionPerformance.NOOP))
.thenReturn(SafeFuture.completedFuture(blockContainerAndMetaData));

assertThat(
validatorApiHandler.createUnsignedBlock(
newSlot, randaoReveal, Optional.empty(), Optional.of(false), Optional.of(ONE)))
newSlot, randaoReveal, Optional.empty(), Optional.of(ONE)))
.isCompleted();

final SignedBeaconBlock block =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ void shouldGetUnBlindedBeaconBlockAsJson() throws Exception {
final BLSSignature signature =
blockContainerAndMetaData.blockContainer().getBlock().getBody().getRandaoReveal();
when(validatorApiChannel.createUnsignedBlock(
eq(UInt64.ONE), eq(signature), any(), any(), any()))
eq(UInt64.ONE), eq(signature), any(), any()))
.thenReturn(SafeFuture.completedFuture(Optional.of(blockContainerAndMetaData)));
Response response = get(signature, ContentTypes.JSON);
assertResponseWithHeaders(
Expand All @@ -101,7 +101,7 @@ void shouldGetUnblindedBeaconBlockAsSsz() throws IOException {
final BLSSignature signature =
blockContainerAndMetaData.blockContainer().getBlock().getBody().getRandaoReveal();
when(validatorApiChannel.createUnsignedBlock(
eq(UInt64.ONE), eq(signature), any(), any(), any()))
eq(UInt64.ONE), eq(signature), any(), any()))
.thenReturn(SafeFuture.completedFuture(Optional.of(blockContainerAndMetaData)));
Response response = get(signature, ContentTypes.OCTET_STREAM);
assertResponseWithHeaders(
Expand All @@ -124,7 +124,7 @@ void shouldGetBlindedBeaconBlockAsJson() throws Exception {
final BLSSignature signature =
blockContainerAndMetaData.blockContainer().getBlock().getBody().getRandaoReveal();
when(validatorApiChannel.createUnsignedBlock(
eq(UInt64.ONE), eq(signature), any(), any(), any()))
eq(UInt64.ONE), eq(signature), any(), any()))
.thenReturn(SafeFuture.completedFuture(Optional.of(blockContainerAndMetaData)));
Response response = get(signature, ContentTypes.JSON);
assertResponseWithHeaders(
Expand All @@ -149,7 +149,7 @@ void shouldGetBlindedBeaconBlockAsSsz() throws IOException {
final BLSSignature signature =
blockContainerAndMetaData.blockContainer().getBlock().getBody().getRandaoReveal();
when(validatorApiChannel.createUnsignedBlock(
eq(UInt64.ONE), eq(signature), any(), any(), any()))
eq(UInt64.ONE), eq(signature), any(), any()))
.thenReturn(SafeFuture.completedFuture(Optional.of(blockContainerAndMetaData)));
Response response = get(signature, ContentTypes.OCTET_STREAM);
assertResponseWithHeaders(
Expand All @@ -173,7 +173,7 @@ void shouldGetUnBlindedBlockContentPostDenebAsJson() throws Exception {
final BLSSignature signature =
blockContainerAndMetaData.blockContainer().getBlock().getBody().getRandaoReveal();
when(validatorApiChannel.createUnsignedBlock(
eq(UInt64.ONE), eq(signature), any(), any(), any()))
eq(UInt64.ONE), eq(signature), any(), any()))
.thenReturn(SafeFuture.completedFuture(Optional.of(blockContainerAndMetaData)));
Response response = get(signature, ContentTypes.JSON);
assertResponseWithHeaders(
Expand All @@ -197,7 +197,7 @@ void shouldGetUnBlindedBlockContentPostDenebAsSsz() throws IOException {
dataStructureUtil.randomBlockContainerAndMetaData(blockContents, ONE);
final BLSSignature signature = blockContents.getBlock().getBody().getRandaoReveal();
when(validatorApiChannel.createUnsignedBlock(
eq(UInt64.ONE), eq(signature), any(), any(), any()))
eq(UInt64.ONE), eq(signature), any(), any()))
.thenReturn(SafeFuture.completedFuture(Optional.of(blockContainerAndMetaData)));
Response response = get(signature, ContentTypes.OCTET_STREAM);
assertResponseWithHeaders(
Expand All @@ -218,7 +218,7 @@ void shouldFailWhenNoBlockProduced() throws IOException {
final BeaconBlock beaconBlock = dataStructureUtil.randomBeaconBlock(ONE);
final BLSSignature signature = beaconBlock.getBlock().getBody().getRandaoReveal();
when(validatorApiChannel.createUnsignedBlock(
eq(UInt64.ONE), eq(signature), any(), any(), any()))
eq(UInt64.ONE), eq(signature), any(), any()))
.thenReturn(SafeFuture.completedFuture(Optional.empty()));
Response response = get(signature, ContentTypes.JSON);
assertThat(response.code()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
Expand Down
Loading
Loading