diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockFactory.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockFactory.java index 3046cb9445c..d1b389dee41 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockFactory.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockFactory.java @@ -47,6 +47,12 @@ public SafeFuture createUnsignedBlock( blockSlotState.getSlot(), newSlot); + System.out.println("****** newSlot " + newSlot); + System.out.println("****** blockSlotState slot " + blockSlotState.getSlot()); + System.out.println( + "****** blockSlotState getLatestBlockHeader slot " + + blockSlotState.getLatestBlockHeader().getSlot()); + // Process empty slots up to the one before the new block slot final UInt64 slotBeforeBlock = newSlot.minus(UInt64.ONE); diff --git a/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/BuilderCircuitBreakerImpl.java b/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/BuilderCircuitBreakerImpl.java index 79c4f001213..74e915b609c 100644 --- a/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/BuilderCircuitBreakerImpl.java +++ b/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/BuilderCircuitBreakerImpl.java @@ -43,15 +43,20 @@ public BuilderCircuitBreakerImpl( @Override public boolean isEngaged(final BeaconState state) { - try { - if (getLatestUniqueBlockRootsCount(state) <= minimumUniqueBlockRootsInWindow) { - return true; - } - } catch (Exception ex) { - LOG.error("Builder circuit breaker check failed. Acting like it has been engaged.", ex); + + final int uniqueBlockRootsCount = getLatestUniqueBlockRootsCount(state); + if (uniqueBlockRootsCount < minimumUniqueBlockRootsInWindow) { + LOG.debug( + "Builder circuit breaker engaged: slot: {}, uniqueBlockRootsCount: {}, window: {}, minimumUniqueBlockRootsInWindow: {}", + state.getSlot(), + uniqueBlockRootsCount, + faultInspectionWindow, + minimumUniqueBlockRootsInWindow); return true; } + LOG.debug("Builder circuit breaker has not engaged."); + return false; } @@ -68,29 +73,33 @@ int getLatestUniqueBlockRootsCount(final BeaconState state) throws IllegalArgume final HashSet uniqueBlockRoots = new HashSet<>(); final SszBytes32Vector blockRoots = state.getBlockRoots(); - // we subtract 1 because blockRoots refers to previous block - // current is getLatestBlockHeader - final UInt64 firstSlotOfInspectionWindow = - state.getSlot().minusMinZero(faultInspectionWindow - 1); + // state slot is the slot we are building for + // thus our fault window will be (inclusive) + // FROM (state_slot-1)-(faultInspectionWindow-1) TO state_slot-1 + + // of which: + // state_slot-1 -> will be represented by getLatestBlockHeader + // FROM (state_slot-1)-(faultInspectionWindow-1) TO state_slot-2 -> to be found in blockRoots + + // (state_slot-1)-(faultInspectionWindow-1) = state_slot-faultInspectionWindow + final UInt64 firstSlotOfInspectionWindow = state.getSlot().minusMinZero(faultInspectionWindow); + final UInt64 lastSlotOfInspectionWindow = state.getSlot().minusMinZero(1); + + // if getLatestBlockHeader is outside the fault window, + // we have definitely missed all blocks so count will be 0 + if (state.getLatestBlockHeader().getSlot().isLessThan(firstSlotOfInspectionWindow)) { + return 0; + } + + uniqueBlockRoots.add(state.getLatestBlockHeader().getRoot()); UInt64 currentSlot = firstSlotOfInspectionWindow; - while (currentSlot.isLessThan(state.getSlot())) { + while (currentSlot.isLessThan(lastSlotOfInspectionWindow)) { final int currentBlockRootIndex = currentSlot.mod(slotsPerHistoricalRoot).intValue(); uniqueBlockRoots.add(blockRoots.getElement(currentBlockRootIndex)); currentSlot = currentSlot.increment(); } - // we add getLatestBlockHeader only if it's slot fall into the window - // otherwise we remove it from the count - if (state - .getLatestBlockHeader() - .getSlot() - .isGreaterThanOrEqualTo(firstSlotOfInspectionWindow)) { - uniqueBlockRoots.add(state.getLatestBlockHeader().getRoot()); - } else { - uniqueBlockRoots.remove(state.getLatestBlockHeader().getRoot()); - } - return uniqueBlockRoots.size(); } } diff --git a/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImpl.java b/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImpl.java index 134e93dd9a9..f45e79665e9 100644 --- a/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImpl.java +++ b/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImpl.java @@ -355,7 +355,7 @@ public SafeFuture builderGetHeader( fallbackReason = FallbackReason.NOT_NEEDED; } else if (isTransitionNotFinalized(executionPayloadContext)) { fallbackReason = FallbackReason.TRANSITION_NOT_FINALIZED; - } else if (builderCircuitBreaker.isEngaged(state)) { + } else if (isCircuitBreakerEngaged(state)) { fallbackReason = FallbackReason.CIRCUIT_BREAKER_ENGAGED; } else if (builderClient.isEmpty()) { fallbackReason = FallbackReason.BUILDER_NOT_CONFIGURED; @@ -528,6 +528,15 @@ private boolean isTransitionNotFinalized(final ExecutionPayloadContext execution return executionPayloadContext.getForkChoiceState().getFinalizedExecutionBlockHash().isZero(); } + private boolean isCircuitBreakerEngaged(final BeaconState state) { + try { + return builderCircuitBreaker.isEngaged(state); + } catch (Exception ex) { + LOG.error("Builder circuit breaker engagement failure. Acting like it has been engaged.", ex); + return true; + } + } + private void markBuilderAsNotAvailable(final String errorMessage) { latestBuilderAvailability.set(false); eventLogger.builderIsOffline(errorMessage); diff --git a/ethereum/executionlayer/src/test/java/tech/pegasys/teku/ethereum/executionlayer/BuilderCircuitBreakerImplTest.java b/ethereum/executionlayer/src/test/java/tech/pegasys/teku/ethereum/executionlayer/BuilderCircuitBreakerImplTest.java index a1a05a30cb1..5e7316dede1 100644 --- a/ethereum/executionlayer/src/test/java/tech/pegasys/teku/ethereum/executionlayer/BuilderCircuitBreakerImplTest.java +++ b/ethereum/executionlayer/src/test/java/tech/pegasys/teku/ethereum/executionlayer/BuilderCircuitBreakerImplTest.java @@ -19,6 +19,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.tuweni.bytes.Bytes32; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import tech.pegasys.teku.infrastructure.ssz.collections.SszBytes32Vector; import tech.pegasys.teku.infrastructure.ssz.schema.collections.SszBytes32VectorSchema; @@ -41,10 +42,18 @@ public class BuilderCircuitBreakerImplTest { private static final int INSPECTION_WINDOW = 10; private static final int ALLOWED_FAULTS = 5; + @Test + @BeforeEach + void check() { + // all tests assume 64 block roots history + assertThat(spec.getGenesisSpec().getSlotsPerHistoricalRoot()).isEqualTo(64); + } + @Test void shouldNotEngage_noMissedSlots() { - final UInt64 slot = UInt64.valueOf(spec.getGenesisSpec().getSlotsPerHistoricalRoot() + 5); + final UInt64 slot = UInt64.valueOf(100); + // fill blockRoots with random roots and have a different latestBlockHeader root too final List blockRoots = Stream.generate(dataStructureUtil::randomBytes32) .limit(spec.getGenesisSpec().getSlotsPerHistoricalRoot()) @@ -58,7 +67,122 @@ void shouldNotEngage_noMissedSlots() { } @Test - void shouldNotEngage_minimalAllowedFaults() {} + void shouldNotEngage_minimalAllowedFaults() { + final UInt64 stateSlot = UInt64.valueOf(69); + + final BeaconBlock latestBlock = dataStructureUtil.randomBeaconBlock(68); + final BeaconBlockHeader latestBlockHeader = BeaconBlockHeader.fromBlock(latestBlock); + + // fill blockRoots [59-63] with 2 unique roots + // fill blockRoots [0-3] (slot 64 to 67) with 2 unique roots + // a different latestBlockHeader root too (slot 68) + // for a total of 5 unique + + final List uniqueRoots = + Stream.generate(dataStructureUtil::randomBytes32).limit(4).collect(Collectors.toList()); + + final List blockRoots = + Stream.concat( + // 4 elements - from slot 64 to 67 + Stream.of( + uniqueRoots.get(0), uniqueRoots.get(1), uniqueRoots.get(0), uniqueRoots.get(1)), + Stream.concat( + // 55 elements + Stream.generate(() -> Bytes32.ZERO) + .limit(spec.getGenesisSpec().getSlotsPerHistoricalRoot() - 9), + // 5 elements - from slot 59 to 63 + Stream.of( + uniqueRoots.get(2), + uniqueRoots.get(3), + uniqueRoots.get(2), + uniqueRoots.get(3), + uniqueRoots.get(2)))) + .collect(Collectors.toList()); + final BeaconState state = prepareState(stateSlot, latestBlockHeader, blockRoots); + + assertThat(builderCircuitBreaker.isEngaged(state)).isFalse(); + assertThat(builderCircuitBreaker.getLatestUniqueBlockRootsCount(state)).isEqualTo(5); + } + + @Test + void shouldEngage_belowAllowedFaults() { + final UInt64 stateSlot = UInt64.valueOf(69); + + final BeaconBlock latestBlock = dataStructureUtil.randomBeaconBlock(67); + final BeaconBlockHeader latestBlockHeader = BeaconBlockHeader.fromBlock(latestBlock); + + // fill blockRoots [59-63] with 2 unique roots + // fill blockRoots [0-3] (slot 64 to 67) with 2 unique roots + // latestBlockHeader same as slot 67 + // for a total of 4 unique + + final List uniqueRoots = + Stream.generate(dataStructureUtil::randomBytes32).limit(4).collect(Collectors.toList()); + + final List blockRoots = + Stream.concat( + // 4 elements - from slot 64 to 67 + Stream.of( + uniqueRoots.get(0), + uniqueRoots.get(0), + uniqueRoots.get(0), + latestBlockHeader.getRoot()), + Stream.concat( + // 55 elements + Stream.generate(() -> Bytes32.ZERO) + .limit(spec.getGenesisSpec().getSlotsPerHistoricalRoot() - 9), + // 5 elements - from slot 59 to 63 + Stream.of( + uniqueRoots.get(2), + uniqueRoots.get(3), + uniqueRoots.get(2), + uniqueRoots.get(3), + uniqueRoots.get(2)))) + .collect(Collectors.toList()); + + final BeaconState state = prepareState(stateSlot, latestBlockHeader, blockRoots); + + assertThat(builderCircuitBreaker.isEngaged(state)).isTrue(); + assertThat(builderCircuitBreaker.getLatestUniqueBlockRootsCount(state)).isEqualTo(4); + } + + @Test + void shouldEngage_belowAllowedFaults_onlyLastBlockHeaderPresent() { + final UInt64 stateSlot = UInt64.valueOf(69); + + final BeaconBlock latestBlock = dataStructureUtil.randomBeaconBlock(59); + final BeaconBlockHeader latestBlockHeader = BeaconBlockHeader.fromBlock(latestBlock); + + // fill blockRoots with random roots and have a different latestBlockHeader root too + final List blockRoots = + Stream.generate(latestBlockHeader::getRoot) + .limit(spec.getGenesisSpec().getSlotsPerHistoricalRoot()) + .collect(Collectors.toList()); + + final BeaconState state = prepareState(stateSlot, latestBlockHeader, blockRoots); + + assertThat(builderCircuitBreaker.isEngaged(state)).isTrue(); + assertThat(builderCircuitBreaker.getLatestUniqueBlockRootsCount(state)).isEqualTo(1); + } + + @Test + void shouldEngage_belowAllowedFaults_lastBlockHeaderWellOff() { + final UInt64 stateSlot = UInt64.valueOf(69); + + final BeaconBlock latestBlock = dataStructureUtil.randomBeaconBlock(40); + final BeaconBlockHeader latestBlockHeader = BeaconBlockHeader.fromBlock(latestBlock); + + // fill blockRoots with random roots and have a different latestBlockHeader root too + final List blockRoots = + Stream.generate(latestBlockHeader::getRoot) + .limit(spec.getGenesisSpec().getSlotsPerHistoricalRoot()) + .collect(Collectors.toList()); + + final BeaconState state = prepareState(stateSlot, latestBlockHeader, blockRoots); + + assertThat(builderCircuitBreaker.isEngaged(state)).isTrue(); + assertThat(builderCircuitBreaker.getLatestUniqueBlockRootsCount(state)).isEqualTo(0); + } private BeaconState prepareState( final UInt64 slot, diff --git a/ethereum/executionlayer/src/test/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImplTest.java b/ethereum/executionlayer/src/test/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImplTest.java index d2c4eaf9896..8aef3dd975a 100644 --- a/ethereum/executionlayer/src/test/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImplTest.java +++ b/ethereum/executionlayer/src/test/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImplTest.java @@ -262,22 +262,7 @@ public void builderGetHeaderGetPayload_shouldReturnHeaderAndPayloadViaEngineOnBu assertThat(executionLayerManager.builderGetHeader(executionPayloadContext, state)) .isCompletedWithValue(header); - // we expect both builder and local engine have been called - verifyBuilderCalled(slot, executionPayloadContext); - verifyEngineCalled(executionPayloadContext); - - final SignedBeaconBlock signedBlindedBeaconBlock = - dataStructureUtil.randomSignedBlindedBeaconBlock(slot); - - // we expect result from the cached payload - assertThat(executionLayerManager.builderGetPayload(signedBlindedBeaconBlock)) - .isCompletedWithValue(payload); - - // we expect no additional calls - verifyNoMoreInteractions(builderClient); - verifyNoMoreInteractions(executionEngineClient); - - verifySourceCounter(Source.BUILDER_LOCAL_EL_FALLBACK, FallbackReason.BUILDER_ERROR); + verifyFallbackToLocalEL(slot, payload, executionPayloadContext, FallbackReason.BUILDER_ERROR); } @Test @@ -303,22 +288,8 @@ public void builderGetHeaderGetPayload_shouldReturnHeaderAndPayloadViaEngineIfBu assertThat(executionLayerManager.builderGetHeader(executionPayloadContext, state)) .isCompletedWithValue(header); - // we expect only local engine have been called - verifyNoInteractions(builderClient); - verifyEngineCalled(executionPayloadContext); - - final SignedBeaconBlock signedBlindedBeaconBlock = - dataStructureUtil.randomSignedBlindedBeaconBlock(slot); - - // we expect result from the cached payload - assertThat(executionLayerManager.builderGetPayload(signedBlindedBeaconBlock)) - .isCompletedWithValue(payload); - - // we expect no additional calls - verifyNoMoreInteractions(builderClient); - verifyNoMoreInteractions(executionEngineClient); - - verifySourceCounter(Source.BUILDER_LOCAL_EL_FALLBACK, FallbackReason.BUILDER_NOT_AVAILABLE); + verifyFallbackToLocalEL( + slot, payload, executionPayloadContext, FallbackReason.BUILDER_NOT_AVAILABLE); } @Test @@ -347,23 +318,8 @@ public void builderGetHeaderGetPayload_shouldReturnHeaderAndPayloadViaEngineIfBu assertThat(executionLayerManager.builderGetHeader(executionPayloadContext, state)) .isCompletedWithValue(header); - // we expect both builder and local engine have been called - verifyBuilderCalled(slot, executionPayloadContext); - verifyEngineCalled(executionPayloadContext); - - final SignedBeaconBlock signedBlindedBeaconBlock = - dataStructureUtil.randomSignedBlindedBeaconBlock(slot); - - // we expect result from the cached payload - assertThat(executionLayerManager.builderGetPayload(signedBlindedBeaconBlock)) - .isCompletedWithValue(payload); - - // we expect no additional calls - verifyNoMoreInteractions(builderClient); - verifyNoMoreInteractions(executionEngineClient); - - verifySourceCounter( - Source.BUILDER_LOCAL_EL_FALLBACK, FallbackReason.BUILDER_HEADER_NOT_AVAILABLE); + verifyFallbackToLocalEL( + slot, payload, executionPayloadContext, FallbackReason.BUILDER_HEADER_NOT_AVAILABLE); } @Test @@ -390,22 +346,8 @@ public void builderGetHeaderGetPayload_shouldReturnHeaderAndPayloadViaEngineIfBu assertThat(executionLayerManager.builderGetHeader(executionPayloadContext, state)) .isCompletedWithValue(header); - // we expect only local engine have been called - verifyNoInteractions(builderClient); - verifyEngineCalled(executionPayloadContext); - - final SignedBeaconBlock signedBlindedBeaconBlock = - dataStructureUtil.randomSignedBlindedBeaconBlock(slot); - - // we expect result from the cached payload - assertThat(executionLayerManager.builderGetPayload(signedBlindedBeaconBlock)) - .isCompletedWithValue(payload); - - // we expect no additional calls - verifyNoMoreInteractions(builderClient); - verifyNoMoreInteractions(executionEngineClient); - - verifySourceCounter(Source.BUILDER_LOCAL_EL_FALLBACK, FallbackReason.TRANSITION_NOT_FINALIZED); + verifyFallbackToLocalEL( + slot, payload, executionPayloadContext, FallbackReason.TRANSITION_NOT_FINALIZED); } @Test @@ -434,22 +376,38 @@ public void builderGetHeaderGetPayload_shouldReturnHeaderAndPayloadViaEngineIfBu assertThat(executionLayerManager.builderGetHeader(executionPayloadContext, state)) .isCompletedWithValue(header); - // we expect only local engine have been called - verifyNoInteractions(builderClient); - verifyEngineCalled(executionPayloadContext); + verifyFallbackToLocalEL( + slot, payload, executionPayloadContext, FallbackReason.CIRCUIT_BREAKER_ENGAGED); + } - final SignedBeaconBlock signedBlindedBeaconBlock = - dataStructureUtil.randomSignedBlindedBeaconBlock(slot); + @Test + public void + builderGetHeaderGetPayload_shouldReturnHeaderAndPayloadViaEngineIfCircuitBreakerThrows() { + setBuilderOnline(); - // we expect result from the cached payload - assertThat(executionLayerManager.builderGetPayload(signedBlindedBeaconBlock)) - .isCompletedWithValue(payload); + final ExecutionPayloadContext executionPayloadContext = + dataStructureUtil.randomPayloadExecutionContext(false, true); + final UInt64 slot = executionPayloadContext.getForkChoiceState().getHeadBlockSlot(); + final BeaconState state = dataStructureUtil.randomBeaconState(slot); - // we expect no additional calls - verifyNoMoreInteractions(builderClient); - verifyNoMoreInteractions(executionEngineClient); + final ExecutionPayload payload = prepareEngineGetPayloadResponse(executionPayloadContext); + + final ExecutionPayloadHeader header = + spec.getGenesisSpec() + .getSchemaDefinitions() + .toVersionBellatrix() + .orElseThrow() + .getExecutionPayloadHeaderSchema() + .createFromExecutionPayload(payload); + + when(builderCircuitBreaker.isEngaged(any())).thenThrow(new RuntimeException("error")); + + // we expect local engine header as result + assertThat(executionLayerManager.builderGetHeader(executionPayloadContext, state)) + .isCompletedWithValue(header); - verifySourceCounter(Source.BUILDER_LOCAL_EL_FALLBACK, FallbackReason.CIRCUIT_BREAKER_ENGAGED); + verifyFallbackToLocalEL( + slot, payload, executionPayloadContext, FallbackReason.CIRCUIT_BREAKER_ENGAGED); } @Test @@ -476,22 +434,8 @@ public void builderGetHeaderGetPayload_shouldReturnHeaderAndPayloadViaEngineIfBu assertThat(executionLayerManager.builderGetHeader(executionPayloadContext, state)) .isCompletedWithValue(header); - // we expect only local engine have been called - verifyNoInteractions(builderClient); - verifyEngineCalled(executionPayloadContext); - - final SignedBeaconBlock signedBlindedBeaconBlock = - dataStructureUtil.randomSignedBlindedBeaconBlock(slot); - - // we expect result from the cached payload - assertThat(executionLayerManager.builderGetPayload(signedBlindedBeaconBlock)) - .isCompletedWithValue(payload); - - // we expect no additional calls - verifyNoMoreInteractions(builderClient); - verifyNoMoreInteractions(executionEngineClient); - - verifySourceCounter(Source.BUILDER_LOCAL_EL_FALLBACK, FallbackReason.VALIDATOR_NOT_REGISTERED); + verifyFallbackToLocalEL( + slot, payload, executionPayloadContext, FallbackReason.VALIDATOR_NOT_REGISTERED); } @Test @@ -558,6 +502,35 @@ private ExecutionPayloadHeader prepareBuilderGetHeaderResponse( return signedBuilderBid.getMessage().getExecutionPayloadHeader(); } + private void verifyFallbackToLocalEL( + final UInt64 slot, + final ExecutionPayload payload, + final ExecutionPayloadContext executionPayloadContext, + final FallbackReason fallbackReason) { + if (fallbackReason == FallbackReason.BUILDER_HEADER_NOT_AVAILABLE + || fallbackReason == FallbackReason.BUILDER_ERROR) { + // we expect both builder and local engine have been called + verifyBuilderCalled(slot, executionPayloadContext); + } else { + // we expect only local engine have been called + verifyNoInteractions(builderClient); + } + verifyEngineCalled(executionPayloadContext); + + final SignedBeaconBlock signedBlindedBeaconBlock = + dataStructureUtil.randomSignedBlindedBeaconBlock(slot); + + // we expect result from the cached payload + assertThat(executionLayerManager.builderGetPayload(signedBlindedBeaconBlock)) + .isCompletedWithValue(payload); + + // we expect no additional calls + verifyNoMoreInteractions(builderClient); + verifyNoMoreInteractions(executionEngineClient); + + verifySourceCounter(Source.BUILDER_LOCAL_EL_FALLBACK, fallbackReason); + } + private ExecutionPayload prepareBuilderGetPayloadResponse( final SignedBeaconBlock signedBlindedBeaconBlock) { diff --git a/services/executionlayer/src/main/java/tech/pegasys/teku/services/executionlayer/ExecutionLayerConfiguration.java b/services/executionlayer/src/main/java/tech/pegasys/teku/services/executionlayer/ExecutionLayerConfiguration.java index 566e11514f1..4ac808c5499 100644 --- a/services/executionlayer/src/main/java/tech/pegasys/teku/services/executionlayer/ExecutionLayerConfiguration.java +++ b/services/executionlayer/src/main/java/tech/pegasys/teku/services/executionlayer/ExecutionLayerConfiguration.java @@ -24,24 +24,38 @@ import tech.pegasys.teku.spec.executionlayer.ExecutionLayerChannel.Version; public class ExecutionLayerConfiguration { + public static final boolean DEFAULT_BUILDER_CIRCUIT_BREAKER_ENABLED = true; + public static final int DEFAULT_BUILDER_CIRCUIT_BREAKER_WINDOW = 32; + public static final int DEFAULT_BUILDER_CIRCUIT_BREAKER_ALLOWED_FAULTS = 8; + + public static final int BUILDER_CIRCUIT_BREAKER_WINDOW_HARD_CAP = 64; private final Spec spec; private final Optional engineEndpoint; private final Version engineVersion; private final Optional engineJwtSecretFile; private final Optional builderEndpoint; + boolean isBuilderCircuitBreakerEnabled; + int builderCircuitBreakerWindow; + int builderCircuitBreakerAllowedFaults; private ExecutionLayerConfiguration( final Spec spec, final Optional engineEndpoint, final Version engineVersion, final Optional engineJwtSecretFile, - final Optional builderEndpoint) { + final Optional builderEndpoint, + final boolean isBuilderCircuitBreakerEnabled, + final int builderCircuitBreakerWindow, + final int builderCircuitBreakerAllowedFaults) { this.spec = spec; this.engineEndpoint = engineEndpoint; this.engineVersion = engineVersion; this.engineJwtSecretFile = engineJwtSecretFile; this.builderEndpoint = builderEndpoint; + this.isBuilderCircuitBreakerEnabled = isBuilderCircuitBreakerEnabled; + this.builderCircuitBreakerWindow = builderCircuitBreakerWindow; + this.builderCircuitBreakerAllowedFaults = builderCircuitBreakerAllowedFaults; } public static Builder builder() { @@ -75,19 +89,42 @@ public Optional getBuilderEndpoint() { return builderEndpoint; } + public boolean isBuilderCircuitBreakerEnabled() { + return isBuilderCircuitBreakerEnabled; + } + + public int getBuilderCircuitBreakerWindow() { + return builderCircuitBreakerWindow; + } + + public int getBuilderCircuitBreakerAllowedFaults() { + return builderCircuitBreakerAllowedFaults; + } + public static class Builder { private Spec spec; private Optional engineEndpoint = Optional.empty(); private Version engineVersion = Version.DEFAULT_VERSION; private Optional engineJwtSecretFile = Optional.empty(); private Optional builderEndpoint = Optional.empty(); + private boolean isBuilderCircuitBreakerEnabled = DEFAULT_BUILDER_CIRCUIT_BREAKER_ENABLED; + private int builderCircuitBreakerWindow = DEFAULT_BUILDER_CIRCUIT_BREAKER_WINDOW; + private int builderCircuitBreakerAllowedFaults = DEFAULT_BUILDER_CIRCUIT_BREAKER_ALLOWED_FAULTS; private Builder() {} public ExecutionLayerConfiguration build() { validateStubEndpoints(); + validateBuilderCircuitBreaker(); return new ExecutionLayerConfiguration( - spec, engineEndpoint, engineVersion, engineJwtSecretFile, builderEndpoint); + spec, + engineEndpoint, + engineVersion, + engineJwtSecretFile, + builderEndpoint, + isBuilderCircuitBreakerEnabled, + builderCircuitBreakerWindow, + builderCircuitBreakerAllowedFaults); } public Builder engineEndpoint(final String engineEndpoint) { @@ -110,6 +147,22 @@ public Builder engineJwtSecretFile(final String jwtSecretFile) { return this; } + public Builder isBuilderCircuitBreakerEnabled(final boolean isBuilderCircuitBreakerEnabled) { + this.isBuilderCircuitBreakerEnabled = isBuilderCircuitBreakerEnabled; + return this; + } + + public Builder builderCircuitBreakerWindow(final int builderCircuitBreakerWindow) { + this.builderCircuitBreakerWindow = builderCircuitBreakerWindow; + return this; + } + + public Builder builderCircuitBreakerAllowedFaults( + final int builderCircuitBreakerAllowedFaults) { + this.builderCircuitBreakerAllowedFaults = builderCircuitBreakerAllowedFaults; + return this; + } + public Builder builderEndpoint(final String builderEndpoint) { this.builderEndpoint = Optional.ofNullable(builderEndpoint); return this; @@ -125,5 +178,13 @@ private void validateStubEndpoints() { engineIsStub == builderIsStub || builderEndpoint.isEmpty(), "mixed configuration with stubbed and non-stubbed execution layer endpoints is not supported"); } + + private void validateBuilderCircuitBreaker() { + if (builderCircuitBreakerWindow > BUILDER_CIRCUIT_BREAKER_WINDOW_HARD_CAP) { + throw new InvalidConfigurationException( + "Builder Circuit Breaker window cannot exceed " + + BUILDER_CIRCUIT_BREAKER_WINDOW_HARD_CAP); + } + } } } diff --git a/services/executionlayer/src/main/java/tech/pegasys/teku/services/executionlayer/ExecutionLayerService.java b/services/executionlayer/src/main/java/tech/pegasys/teku/services/executionlayer/ExecutionLayerService.java index d9df432fd61..1b700bce6d5 100644 --- a/services/executionlayer/src/main/java/tech/pegasys/teku/services/executionlayer/ExecutionLayerService.java +++ b/services/executionlayer/src/main/java/tech/pegasys/teku/services/executionlayer/ExecutionLayerService.java @@ -31,6 +31,7 @@ import tech.pegasys.teku.ethereum.executionclient.rest.RestClientProvider; import tech.pegasys.teku.ethereum.executionclient.web3j.ExecutionWeb3jClientProvider; import tech.pegasys.teku.ethereum.executionlayer.BuilderBidValidatorImpl; +import tech.pegasys.teku.ethereum.executionlayer.BuilderCircuitBreaker; import tech.pegasys.teku.ethereum.executionlayer.BuilderCircuitBreakerImpl; import tech.pegasys.teku.ethereum.executionlayer.ExecutionLayerManager; import tech.pegasys.teku.ethereum.executionlayer.ExecutionLayerManagerImpl; @@ -125,6 +126,14 @@ public static ExecutionLayerService create( timeProvider, metricsSystem)); + final BuilderCircuitBreaker builderCircuitBreaker = + config.isBuilderCircuitBreakerEnabled() + ? new BuilderCircuitBreakerImpl( + config.getSpec(), + config.getBuilderCircuitBreakerWindow(), + config.getBuilderCircuitBreakerAllowedFaults()) + : BuilderCircuitBreaker.NOOP; + executionLayerManager = ExecutionLayerManagerImpl.create( EVENT_LOG, @@ -133,7 +142,7 @@ public static ExecutionLayerService create( config.getSpec(), metricsSystem, new BuilderBidValidatorImpl(EVENT_LOG), - new BuilderCircuitBreakerImpl(config.getSpec(), 32, 10)); + builderCircuitBreaker); } return new ExecutionLayerService( diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/ExecutionLayerOptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/ExecutionLayerOptions.java index 8a3d22eb313..98702bf6928 100644 --- a/teku/src/main/java/tech/pegasys/teku/cli/options/ExecutionLayerOptions.java +++ b/teku/src/main/java/tech/pegasys/teku/cli/options/ExecutionLayerOptions.java @@ -14,8 +14,12 @@ package tech.pegasys.teku.cli.options; import static tech.pegasys.teku.config.TekuConfiguration.Builder; +import static tech.pegasys.teku.services.executionlayer.ExecutionLayerConfiguration.DEFAULT_BUILDER_CIRCUIT_BREAKER_ALLOWED_FAULTS; +import static tech.pegasys.teku.services.executionlayer.ExecutionLayerConfiguration.DEFAULT_BUILDER_CIRCUIT_BREAKER_ENABLED; +import static tech.pegasys.teku.services.executionlayer.ExecutionLayerConfiguration.DEFAULT_BUILDER_CIRCUIT_BREAKER_WINDOW; import picocli.CommandLine; +import picocli.CommandLine.Help.Visibility; import picocli.CommandLine.Option; import tech.pegasys.teku.spec.executionlayer.ExecutionLayerChannel.Version; @@ -51,16 +55,47 @@ public class ExecutionLayerOptions { names = {"--builder-endpoint"}, paramLabel = "", description = "URL for an external Builder node (optional).", + showDefaultValue = Visibility.ALWAYS, arity = "1") private String builderEndpoint = null; + @Option( + names = {"--Xbuilder-circuit-breaker-enabled"}, + paramLabel = "", + description = "Enables Circuit Breaker logic for builder usage.", + arity = "1", + showDefaultValue = Visibility.ALWAYS, + hidden = true) + private boolean builderCircuitBreakerEnabled = DEFAULT_BUILDER_CIRCUIT_BREAKER_ENABLED; + + @Option( + names = {"--Xbuilder-circuit-breaker-window"}, + paramLabel = "", + description = "Circuit Breaker fault inspection window.", + arity = "1", + showDefaultValue = Visibility.ALWAYS, + hidden = true) + private int builderCircuitBreakerWindow = DEFAULT_BUILDER_CIRCUIT_BREAKER_WINDOW; + + @Option( + names = {"--Xbuilder-circuit-breaker-allowed-faults"}, + paramLabel = "", + description = + "Circuit Breaker maximum allowed faults (missing block) within the specified inspection window.", + arity = "1", + hidden = true) + private int builderCircuitBreakerAllowedFaults = DEFAULT_BUILDER_CIRCUIT_BREAKER_ALLOWED_FAULTS; + public void configure(final Builder builder) { builder.executionLayer( b -> b.engineEndpoint(executionEngineEndpoint) .engineVersion(executionEngineVersion) .engineJwtSecretFile(engineJwtSecretFile) - .builderEndpoint(builderEndpoint)); + .builderEndpoint(builderEndpoint) + .isBuilderCircuitBreakerEnabled(builderCircuitBreakerEnabled) + .builderCircuitBreakerWindow(builderCircuitBreakerWindow) + .builderCircuitBreakerAllowedFaults(builderCircuitBreakerAllowedFaults)); depositOptions.configure(builder); } } diff --git a/teku/src/test/java/tech/pegasys/teku/cli/options/ExecutionLayerOptionsTest.java b/teku/src/test/java/tech/pegasys/teku/cli/options/ExecutionLayerOptionsTest.java index 2db6b7f5929..1fc923b6471 100644 --- a/teku/src/test/java/tech/pegasys/teku/cli/options/ExecutionLayerOptionsTest.java +++ b/teku/src/test/java/tech/pegasys/teku/cli/options/ExecutionLayerOptionsTest.java @@ -15,6 +15,9 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static tech.pegasys.teku.services.executionlayer.ExecutionLayerConfiguration.BUILDER_CIRCUIT_BREAKER_WINDOW_HARD_CAP; +import static tech.pegasys.teku.services.executionlayer.ExecutionLayerConfiguration.DEFAULT_BUILDER_CIRCUIT_BREAKER_ALLOWED_FAULTS; +import static tech.pegasys.teku.services.executionlayer.ExecutionLayerConfiguration.DEFAULT_BUILDER_CIRCUIT_BREAKER_WINDOW; import org.junit.jupiter.api.Test; import tech.pegasys.teku.cli.AbstractBeaconNodeCommandTest; @@ -85,6 +88,51 @@ public void shouldAcceptEngineAndBuilderEndpointIfSpecEnablesBellatrix() { .isEqualTo(config); } + @Test + public void shouldBuilderCircuitBreakerEnabledByDefault() { + final String[] args = {}; + final TekuConfiguration config = getTekuConfigurationFromArguments(args); + + assertThat(config.executionLayer().isBuilderCircuitBreakerEnabled()).isTrue(); + assertThat(config.executionLayer().getBuilderCircuitBreakerWindow()) + .isEqualTo(DEFAULT_BUILDER_CIRCUIT_BREAKER_WINDOW); + assertThat(config.executionLayer().getBuilderCircuitBreakerAllowedFaults()) + .isEqualTo(DEFAULT_BUILDER_CIRCUIT_BREAKER_ALLOWED_FAULTS); + } + + @Test + public void shouldAcceptBuilderCircuitBreakerParams() { + final String[] args = { + "--Xbuilder-circuit-breaker-enabled", + "false", + "--Xbuilder-circuit-breaker-window", + "40", + "--Xbuilder-circuit-breaker-allowed-faults", + "2" + }; + final TekuConfiguration config = getTekuConfigurationFromArguments(args); + + assertThat(config.executionLayer().isBuilderCircuitBreakerEnabled()).isFalse(); + assertThat(config.executionLayer().getBuilderCircuitBreakerWindow()).isEqualTo(40); + assertThat(config.executionLayer().getBuilderCircuitBreakerAllowedFaults()).isEqualTo(2); + } + + @Test + public void shouldThrowWithBuilderCircuitBreakerWindowTooLarge() { + assertThatThrownBy( + () -> + createConfigBuilder() + .executionLayer( + b -> + b.builderCircuitBreakerWindow( + BUILDER_CIRCUIT_BREAKER_WINDOW_HARD_CAP + 1) + .build())) + .isInstanceOf(InvalidConfigurationException.class) + .hasMessage( + "Builder Circuit Breaker window cannot exceed " + + BUILDER_CIRCUIT_BREAKER_WINDOW_HARD_CAP); + } + @Test public void shouldReportEEDisabledIfEndpointNotSpecified() { final TekuConfiguration config = getTekuConfigurationFromArguments("--network=minimal");