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.

Sign up for GitHub

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

builder fallback improvements #5935

Merged
merged 4 commits into from
Jul 15, 2022
Merged

builder fallback improvements #5935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fd01bab
builder fallback improvements
tbenr Jul 14, 2022
c37eb5a
fix compilation
tbenr Jul 14, 2022
c26431d
Merge branch 'master' into fallback_reason_improvements
tbenr Jul 14, 2022
6095a89
Merge branch 'master' into fallback_reason_improvements
tbenr Jul 15, 2022
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
12 changes: 2 additions & 10 deletions .../src/main/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.tuweni.bytes.Bytes32;
import tech.pegasys.teku.bls.BLSSignature;
import tech.pegasys.teku.infrastructure.async.SafeFuture;
Expand All @@ -47,8 +45,6 @@
import tech.pegasys.teku.statetransition.synccommittee.SyncCommitteeContributionPool;

public class BlockOperationSelectorFactory {
private static final Logger LOG = LogManager.getLogger();

private final Spec spec;
private final AggregatingAttestationPool attestationPool;
private final OperationPool<AttesterSlashing> attesterSlashingPool;
Expand Down Expand Up @@ -153,18 +149,14 @@ public Consumer<BeaconBlockBodyBuilder> createSelector(
.getExecutionPayloadHeaderSchema()
.getHeaderOfDefaultPayload(),
(executionPayloadContext) -> {
final boolean forceLocalFallback =
final boolean transitionNotFinalized =
executionPayloadContext
.getForkChoiceState()
.getFinalizedExecutionBlockHash()
.isZero();

if (forceLocalFallback) {
LOG.info(
"Merge transition not finalized: forcing block production using local execution engine");
}
return executionLayerChannel.builderGetHeader(
executionPayloadContext, blockSlotState, forceLocalFallback);
executionPayloadContext, blockSlotState, transitionNotFinalized);
}));
return;
}
Expand Down
18 changes: 13 additions & 5 deletions ...er/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Optional;
import java.util.concurrent.ConcurrentSkipListMap;
import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.tuweni.bytes.Bytes32;
Expand Down Expand Up @@ -337,7 +338,7 @@ public SafeFuture<Void> builderRegisterValidators(
public SafeFuture<ExecutionPayloadHeader> builderGetHeader(
final ExecutionPayloadContext executionPayloadContext,
final BeaconState state,
final boolean forceLocalFallback) {
final boolean transitionNotFinalized) {
final UInt64 slot = state.getSlot();

final SafeFuture<ExecutionPayload> localExecutionPayload =
Expand All @@ -348,8 +349,12 @@ public SafeFuture<ExecutionPayloadHeader> builderGetHeader(

// fallback conditions
final FallbackReason fallbackReason;
if (forceLocalFallback) {
fallbackReason = FallbackReason.FORCED;
if (executionBuilderClient.isEmpty() && validatorRegistration.isEmpty()) {
fallbackReason = FallbackReason.NOT_NEEDED;
} else if (transitionNotFinalized) {
fallbackReason = FallbackReason.TRANSITION_NOT_FINALIZED;
} else if (executionBuilderClient.isEmpty()) {
fallbackReason = FallbackReason.BUILDER_NOT_CONFIGURED;
} else if (!isBuilderAvailable()) {
fallbackReason = FallbackReason.BUILDER_NOT_AVAILABLE;
} else if (validatorRegistration.isEmpty()) {
Expand Down Expand Up @@ -521,7 +526,8 @@ private void markBuilderAsNotAvailable(final String errorMessage) {
}

private void logFallbackToLocalExecutionPayload(final FallbackData fallbackData) {
LOG.info(
LOG.log(
fallbackData.reason == FallbackReason.NOT_NEEDED ? Level.DEBUG : Level.INFO,
"Falling back to locally produced execution payload (Block Number {}, Block Hash = {}, Fallback Reason = {})",
fallbackData.executionPayload.getBlockNumber(),
fallbackData.executionPayload.getBlockHash(),
Expand Down Expand Up @@ -581,9 +587,11 @@ public String toString() {

// Metric - fallback "reason" label values
protected enum FallbackReason {
NOT_NEEDED("not_needed"),
VALIDATOR_NOT_REGISTERED("validator_not_registered"),
FORCED("forced"),
TRANSITION_NOT_FINALIZED("transition_not_finalized"),
BUILDER_NOT_AVAILABLE("builder_not_available"),
BUILDER_NOT_CONFIGURED("builder_not_configured"),
BUILDER_HEADER_NOT_AVAILABLE("builder_header_not_available"),
BUILDER_ERROR("builder_error"),
NONE("");
Expand Down
2 changes: 1 addition & 1 deletion ...rc/test/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ public void builderGetHeaderGetPayload_shouldReturnHeaderAndPayloadViaEngineIfBu
verifyNoMoreInteractions(executionBuilderClient);
verifyNoMoreInteractions(executionEngineClient);

verifySourceCounter(Source.BUILDER_LOCAL_EL_FALLBACK, FallbackReason.FORCED);
verifySourceCounter(Source.BUILDER_LOCAL_EL_FALLBACK, FallbackReason.TRANSITION_NOT_FINALIZED);
}

@Test
Expand Down
4 changes: 2 additions & 2 deletions ethereum/spec/src/main/java/tech/pegasys/teku/spec/executionlayer/ExecutionLayerChannel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public SafeFuture<Void> builderRegisterValidators(
public SafeFuture<ExecutionPayloadHeader> builderGetHeader(
final ExecutionPayloadContext executionPayloadContext,
final BeaconState state,
final boolean forceLocalFallback) {
final boolean transitionNotFinalized) {
return SafeFuture.completedFuture(null);
}

Expand Down Expand Up @@ -113,7 +113,7 @@ SafeFuture<Void> builderRegisterValidators(
SafeFuture<ExecutionPayloadHeader> builderGetHeader(
ExecutionPayloadContext executionPayloadContext,
BeaconState state,
boolean forceLocalFallback);
boolean transitionNotFinalized);

SafeFuture<ExecutionPayload> builderGetPayload(SignedBeaconBlock signedBlindedBeaconBlock);

Expand Down
2 changes: 1 addition & 1 deletion ...m/spec/src/main/java/tech/pegasys/teku/spec/executionlayer/ExecutionLayerChannelStub.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ public SafeFuture<Void> builderRegisterValidators(
public SafeFuture<ExecutionPayloadHeader> builderGetHeader(
final ExecutionPayloadContext executionPayloadContext,
final BeaconState state,
final boolean forceLocalFallback) {
final boolean transitionNotFinalized) {
final UInt64 slot = state.getSlot();
LOG.info(
"getPayloadHeader: payloadId: {} slot: {} ... delegating to getPayload ...",
Expand Down