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

Introduce option to use local payload instead of builder #6878

Merged
merged 11 commits into from
Mar 2, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
public class ExecutionBuilderModule {

private static final Logger LOG = LogManager.getLogger();
private static final int HUNDRED_PERCENT = 100;

private final Spec spec;
private final AtomicBoolean latestBuilderAvailability;
Expand All @@ -58,21 +59,24 @@ public class ExecutionBuilderModule {
private final BuilderCircuitBreaker builderCircuitBreaker;
private final Optional<BuilderClient> builderClient;
private final EventLogger eventLogger;
private final Optional<Integer> builderBidChallengePercentage;

public ExecutionBuilderModule(
final Spec spec,
final ExecutionLayerManagerImpl executionLayerManager,
final BuilderBidValidator builderBidValidator,
final BuilderCircuitBreaker builderCircuitBreaker,
final Optional<BuilderClient> builderClient,
final EventLogger eventLogger) {
final EventLogger eventLogger,
final Optional<Integer> builderBidChallengePercentage) {
this.spec = spec;
this.latestBuilderAvailability = new AtomicBoolean(builderClient.isPresent());
this.executionLayerManager = executionLayerManager;
this.builderBidValidator = builderBidValidator;
this.builderCircuitBreaker = builderCircuitBreaker;
this.builderClient = builderClient;
this.eventLogger = eventLogger;
this.builderBidChallengePercentage = builderBidChallengePercentage;
}

private Optional<SafeFuture<HeaderWithFallbackData>> validateBuilderGetHeader(
Expand Down Expand Up @@ -164,10 +168,18 @@ public SafeFuture<HeaderWithFallbackData> builderGetHeader(
.map(ExecutionPayloadWithValue::getValue)
.orElse(UInt256.ZERO);
logReceivedBuilderBid(signedBuilderBid.getMessage());
if (signedBuilderBid.getMessage().getValue().lessOrEqualThan(localPayloadValue)) {

if (isLocalPayloadValueWinning(signedBuilderBid, localPayloadValue)) {
LOG.info(
"The local execution payload value ({}) was awarded the block over the builder payload ({}), "
+ "{}% challenge configured.",
signedBuilderBid.getMessage().getValue().toDecimalString(),
localPayloadValue.toDecimalString(),
builderBidChallengePercentage);
return getResultFromLocalExecutionPayload(
localExecutionPayload, slot, FallbackReason.LOCAL_BLOCK_VALUE_HIGHER);
localExecutionPayload, slot, FallbackReason.LOCAL_BLOCK_VALUE_WON);
}

final Optional<ExecutionPayload> localPayload =
maybeLocalExecutionPayload.map(ExecutionPayloadWithValue::getExecutionPayload);
return getResultFromSignedBuilderBid(
Expand All @@ -184,6 +196,17 @@ public SafeFuture<HeaderWithFallbackData> builderGetHeader(
});
}

/** 1 ETH is 10^18 wei, Uint256 max is more than 10^77 */
private boolean isLocalPayloadValueWinning(
final SignedBuilderBid signedBuilderBid, final UInt256 localPayloadValue) {
return builderBidChallengePercentage.isPresent()
&& signedBuilderBid
.getMessage()
.getValue()
.multiply(builderBidChallengePercentage.get())
.lessOrEqualThan(localPayloadValue.multiply(HUNDRED_PERCENT));
}

private SafeFuture<HeaderWithFallbackData> getResultFromSignedBuilderBid(
final SignedBuilderBid signedBuilderBid,
final BeaconState state,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ public static ExecutionLayerManagerImpl create(
final MetricsSystem metricsSystem,
final BuilderBidValidator builderBidValidator,
final BuilderCircuitBreaker builderCircuitBreaker,
final BlobsBundleValidator blobsBundleValidator) {
final BlobsBundleValidator blobsBundleValidator,
final Optional<Integer> builderBidChallengePercentage) {
final LabelledMetric<Counter> executionPayloadSourceCounter =
metricsSystem.createLabelledCounter(
TekuMetricCategory.BEACON,
Expand Down Expand Up @@ -110,7 +111,8 @@ public static ExecutionLayerManagerImpl create(
builderBidValidator,
builderCircuitBreaker,
executionPayloadSourceCounter,
blobsBundleValidator);
blobsBundleValidator,
builderBidChallengePercentage);
}

public static ExecutionLayerManagerImpl create(
Expand All @@ -121,7 +123,8 @@ public static ExecutionLayerManagerImpl create(
final MetricsSystem metricsSystem,
final BuilderBidValidator builderBidValidator,
final BuilderCircuitBreaker builderCircuitBreaker,
final BlobsBundleValidator blobsBundleValidator) {
final BlobsBundleValidator blobsBundleValidator,
final Optional<Integer> builderBidChallengePercentage) {
final ExecutionClientHandler executionClientHandler;

if (spec.isMilestoneSupported(SpecMilestone.DENEB)) {
Expand All @@ -140,7 +143,8 @@ public static ExecutionLayerManagerImpl create(
metricsSystem,
builderBidValidator,
builderCircuitBreaker,
blobsBundleValidator);
blobsBundleValidator,
builderBidChallengePercentage);
}

public static ExecutionEngineClient createEngineClient(
Expand Down Expand Up @@ -181,14 +185,21 @@ private ExecutionLayerManagerImpl(
final BuilderBidValidator builderBidValidator,
final BuilderCircuitBreaker builderCircuitBreaker,
final LabelledMetric<Counter> executionPayloadSourceCounter,
final BlobsBundleValidator blobsBundleValidator) {
final BlobsBundleValidator blobsBundleValidator,
final Optional<Integer> builderBidChallengePercentage) {
this.executionClientHandler = executionClientHandler;
this.spec = spec;
this.blobsBundleValidator = blobsBundleValidator;
this.executionPayloadSourceCounter = executionPayloadSourceCounter;
this.executionBuilderModule =
new ExecutionBuilderModule(
spec, this, builderBidValidator, builderCircuitBreaker, builderClient, eventLogger);
spec,
this,
builderBidValidator,
builderCircuitBreaker,
builderClient,
eventLogger,
builderBidChallengePercentage);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ private void verifyFallbackToLocalEL(
final ExecutionPayload executionPayload = fallbackData.getExecutionPayload();
if (fallbackReason == FallbackReason.BUILDER_HEADER_NOT_AVAILABLE
|| fallbackReason == FallbackReason.BUILDER_ERROR
|| fallbackReason == FallbackReason.LOCAL_BLOCK_VALUE_HIGHER) {
|| fallbackReason == FallbackReason.LOCAL_BLOCK_VALUE_WON) {
// we expect both builder and local engine have been called
verifyBuilderCalled(slot, executionPayloadContext);
} else {
Expand Down Expand Up @@ -469,7 +469,8 @@ private ExecutionLayerManagerImpl createExecutionLayerChannelImpl(
? new BuilderBidValidatorImpl(eventLogger)
: BuilderBidValidator.NOOP,
builderCircuitBreaker,
BlobsBundleValidator.NOOP);
BlobsBundleValidator.NOOP,
Optional.of(100));
}

private void updateBuilderStatus(SafeFuture<Response<Void>> builderClientResponse, UInt64 slot) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import tech.pegasys.teku.spec.Spec;
import tech.pegasys.teku.spec.TestSpecFactory;
import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock;
import tech.pegasys.teku.spec.datastructures.builder.BuilderBid;
import tech.pegasys.teku.spec.datastructures.builder.SignedBuilderBid;
import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayload;
import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadContext;
Expand Down Expand Up @@ -196,7 +197,7 @@ public void builderGetHeaderGetPayload_shouldReturnHeaderAndPayloadViaBuilder()
final BeaconState state = dataStructureUtil.randomBeaconState(slot);

final ExecutionPayloadHeader header =
prepareBuilderGetHeaderResponse(executionPayloadContext, false);
prepareBuilderGetHeaderResponse(executionPayloadContext, false).getExecutionPayloadHeader();
prepareEngineGetPayloadResponse(executionPayloadContext, UInt256.ZERO, slot);

// we expect result from the builder
Expand Down Expand Up @@ -234,9 +235,46 @@ public void builderGetHeaderGetPayload_shouldReturnHeaderAndPayloadViaEngineWhen
final UInt64 slot = executionPayloadContext.getForkChoiceState().getHeadBlockSlot();
final BeaconState state = dataStructureUtil.randomBeaconState(slot);

prepareBuilderGetHeaderResponse(executionPayloadContext, false);
final UInt256 builderValue =
prepareBuilderGetHeaderResponse(executionPayloadContext, false).getValue();
final ExecutionPayload localExecutionPayload =
prepareEngineGetPayloadResponse(executionPayloadContext, builderValue, slot);

// we expect result from the local engine
final ExecutionPayloadHeader expectedHeader =
spec.getGenesisSpec()
.getSchemaDefinitions()
.toVersionBellatrix()
.orElseThrow()
.getExecutionPayloadHeaderSchema()
.createFromExecutionPayload(localExecutionPayload);

// we expect local engine header as result
final HeaderWithFallbackData expectedResult =
HeaderWithFallbackData.create(
expectedHeader,
new FallbackData(localExecutionPayload, FallbackReason.LOCAL_BLOCK_VALUE_WON));
assertThat(executionLayerManager.builderGetHeader(executionPayloadContext, state))
.isCompletedWithValue(expectedResult);
verifyFallbackToLocalEL(slot, executionPayloadContext, expectedResult);
}

@Test
public void builderGetHeaderGetPayload_shouldReturnEnginePayloadWhenValueLowerButChallengeWon() {
// Setup requires local payload to have at lest 50% value of builder's to win
executionLayerManager = createExecutionLayerChannelImpl(true, false, Optional.of(50));
setBuilderOnline();

final ExecutionPayloadContext executionPayloadContext =
dataStructureUtil.randomPayloadExecutionContext(false, true);
final UInt64 slot = executionPayloadContext.getForkChoiceState().getHeadBlockSlot();
final BeaconState state = dataStructureUtil.randomBeaconState(slot);

final UInt256 builderValue =
prepareBuilderGetHeaderResponse(executionPayloadContext, false).getValue();
final ExecutionPayload localExecutionPayload =
prepareEngineGetPayloadResponse(executionPayloadContext, UInt256.MAX_VALUE, slot);
prepareEngineGetPayloadResponse(
executionPayloadContext, builderValue.multiply(51).divide(100), slot);
rolfyone marked this conversation as resolved.
Show resolved Hide resolved

// we expect result from the local engine
final ExecutionPayloadHeader expectedHeader =
Expand All @@ -251,12 +289,57 @@ public void builderGetHeaderGetPayload_shouldReturnHeaderAndPayloadViaEngineWhen
final HeaderWithFallbackData expectedResult =
HeaderWithFallbackData.create(
expectedHeader,
new FallbackData(localExecutionPayload, FallbackReason.LOCAL_BLOCK_VALUE_HIGHER));
new FallbackData(localExecutionPayload, FallbackReason.LOCAL_BLOCK_VALUE_WON));
assertThat(executionLayerManager.builderGetHeader(executionPayloadContext, state))
.isCompletedWithValue(expectedResult);
verifyFallbackToLocalEL(slot, executionPayloadContext, expectedResult);
}

@Test
public void builderGetHeaderGetPayload_shouldReturnBuilderPayloadWhenBuilderWonChallenge() {
// Setup requires local payload to have at lest 50% value of builder's to win
executionLayerManager = createExecutionLayerChannelImpl(true, false, Optional.of(50));
setBuilderOnline();

final ExecutionPayloadContext executionPayloadContext =
dataStructureUtil.randomPayloadExecutionContext(false, true);
final UInt64 slot = executionPayloadContext.getForkChoiceState().getHeadBlockSlot();
final BeaconState state = dataStructureUtil.randomBeaconState(slot);

final BuilderBid builderBid = prepareBuilderGetHeaderResponse(executionPayloadContext, false);
prepareEngineGetPayloadResponse(
executionPayloadContext, builderBid.getValue().multiply(49).divide(100), slot);

// we expect result from the builder
final ExecutionPayloadHeader builderHeader = builderBid.getExecutionPayloadHeader();
final HeaderWithFallbackData expectedResult = HeaderWithFallbackData.create(builderHeader);
assertThat(executionLayerManager.builderGetHeader(executionPayloadContext, state))
.isCompletedWithValue(expectedResult);
}

@Test
public void builderGetHeaderGetPayload_shouldReturnBuilderPayloadWhenBuilderChallengeIsNever() {
// Setup will always ignore local payload in favor of Builder bid
executionLayerManager = createExecutionLayerChannelImpl(true, false, Optional.empty());
setBuilderOnline();

final ExecutionPayloadContext executionPayloadContext =
dataStructureUtil.randomPayloadExecutionContext(false, true);
final UInt64 slot = executionPayloadContext.getForkChoiceState().getHeadBlockSlot();
final BeaconState state = dataStructureUtil.randomBeaconState(slot);

final BuilderBid builderBid = prepareBuilderGetHeaderResponse(executionPayloadContext, false);
prepareEngineGetPayloadResponse(
// something tasty, but we should ignore it
executionPayloadContext, builderBid.getValue().multiply(100), slot);

// we expect result from the builder
final ExecutionPayloadHeader builderHeader = builderBid.getExecutionPayloadHeader();
final HeaderWithFallbackData expectedResult = HeaderWithFallbackData.create(builderHeader);
assertThat(executionLayerManager.builderGetHeader(executionPayloadContext, state))
.isCompletedWithValue(expectedResult);
}

@Test
public void builderGetHeaderGetPayload_shouldReturnHeaderAndPayloadViaEngineOnBuilderFailure() {
setBuilderOnline();
Expand Down Expand Up @@ -577,7 +660,7 @@ void onSlot_shouldCleanUpFallbackCache() {
verifyNoMoreInteractions(executionClientHandler);
}

private ExecutionPayloadHeader prepareBuilderGetHeaderResponse(
private BuilderBid prepareBuilderGetHeaderResponse(
final ExecutionPayloadContext executionPayloadContext, final boolean prepareEmptyResponse) {
final UInt64 slot = executionPayloadContext.getForkChoiceState().getHeadBlockSlot();

Expand All @@ -599,7 +682,7 @@ private ExecutionPayloadHeader prepareBuilderGetHeaderResponse(
.orElseThrow(),
executionPayloadContext.getParentHash());

return signedBuilderBid.getMessage().getExecutionPayloadHeader();
return signedBuilderBid.getMessage();
}

private void verifyFallbackToLocalEL(
Expand All @@ -612,7 +695,7 @@ private void verifyFallbackToLocalEL(
final ExecutionPayload executionPayload = fallbackData.getExecutionPayload();
if (fallbackReason == FallbackReason.BUILDER_HEADER_NOT_AVAILABLE
|| fallbackReason == FallbackReason.BUILDER_ERROR
|| fallbackReason == FallbackReason.LOCAL_BLOCK_VALUE_HIGHER) {
|| fallbackReason == FallbackReason.LOCAL_BLOCK_VALUE_WON) {
// we expect both builder and local engine have been called
verifyBuilderCalled(slot, executionPayloadContext);
} else {
Expand Down Expand Up @@ -683,6 +766,14 @@ private ExecutionPayload prepareEngineGetPayloadResponse(

private ExecutionLayerManagerImpl createExecutionLayerChannelImpl(
final boolean builderEnabled, final boolean builderValidatorEnabled) {
return createExecutionLayerChannelImpl(
builderEnabled, builderValidatorEnabled, Optional.of(100));
}

private ExecutionLayerManagerImpl createExecutionLayerChannelImpl(
final boolean builderEnabled,
final boolean builderValidatorEnabled,
final Optional<Integer> builderBidChallengePercentaqe) {
when(builderCircuitBreaker.isEngaged(any())).thenReturn(false);
return ExecutionLayerManagerImpl.create(
eventLogger,
Expand All @@ -694,7 +785,8 @@ private ExecutionLayerManagerImpl createExecutionLayerChannelImpl(
? new BuilderBidValidatorImpl(eventLogger)
: BuilderBidValidator.NOOP,
builderCircuitBreaker,
BlobsBundleValidator.NOOP);
BlobsBundleValidator.NOOP,
builderBidChallengePercentaqe);
}

private void updateBuilderStatus(final SafeFuture<Response<Void>> builderClientResponse) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public enum FallbackReason {
BUILDER_NOT_AVAILABLE("builder_not_available"),
BUILDER_NOT_CONFIGURED("builder_not_configured"),
BUILDER_HEADER_NOT_AVAILABLE("builder_header_not_available"),
LOCAL_BLOCK_VALUE_HIGHER("local_block_value_higher"),
LOCAL_BLOCK_VALUE_WON("local_block_value_won"),
BUILDER_ERROR("builder_error"),
NONE("");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,11 @@ public BuilderBid randomBuilderBid(final BLSPublicKey builderPublicKey) {
return SchemaDefinitionsBellatrix.required(spec.getGenesisSchemaDefinitions())
.getBuilderBidSchema()
.create(
randomExecutionPayloadHeader(spec.getGenesisSpec()), randomUInt256(), builderPublicKey);
randomExecutionPayloadHeader(spec.getGenesisSpec()),
// 1 ETH is 10^18 wei, Uint256 max is more than 10^77, so just to avoid overflows in
// computation
randomUInt256().divide(1000),
builderPublicKey);
}

public BuilderBid randomBuilderBid(final Bytes32 withdrawalsRoot) {
Expand Down
Loading