From a86e1480a140e74252e4e757298a5e873d55fa00 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Tue, 15 Aug 2023 21:49:29 +0200 Subject: [PATCH 01/18] Avoid registration of non-active validators --- .../response/v1/beacon/ValidatorStatus.java | 6 + .../client/ValidatorClientService.java | 3 +- .../client/ValidatorRegistrator.java | 80 +++++++----- .../client/ValidatorRegistratorTest.java | 119 +++++++++++++++--- 4 files changed, 162 insertions(+), 46 deletions(-) diff --git a/data/serializer/src/main/java/tech/pegasys/teku/api/response/v1/beacon/ValidatorStatus.java b/data/serializer/src/main/java/tech/pegasys/teku/api/response/v1/beacon/ValidatorStatus.java index df0204d89ec..ba235663e81 100644 --- a/data/serializer/src/main/java/tech/pegasys/teku/api/response/v1/beacon/ValidatorStatus.java +++ b/data/serializer/src/main/java/tech/pegasys/teku/api/response/v1/beacon/ValidatorStatus.java @@ -37,4 +37,10 @@ public enum ValidatorStatus { public boolean hasExited() { return hasExited; } + + public boolean isActive() { + return this.equals(active_ongoing) + || this.equals(active_exiting) + || this.equals(active_slashed); + } } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java index 81978cf64cf..cb9262b8ab6 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java @@ -181,7 +181,8 @@ public static ValidatorClientService create( proposerConfigManager.get(), new ValidatorRegistrationBatchSender( validatorConfig.getBuilderRegistrationSendingBatchSize(), - validatorApiChannel))); + validatorApiChannel), + validatorApiChannel)); } else { proposerConfigManager = Optional.empty(); } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java index d3151a53655..0a12f178288 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java @@ -16,6 +16,7 @@ import static tech.pegasys.teku.infrastructure.logging.ValidatorLogger.VALIDATOR_LOGGER; import com.google.common.collect.Maps; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -24,6 +25,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.logging.log4j.LogManager; @@ -40,6 +42,7 @@ import tech.pegasys.teku.spec.datastructures.builder.ValidatorRegistration; import tech.pegasys.teku.spec.schemas.ApiSchemas; import tech.pegasys.teku.spec.signatures.Signer; +import tech.pegasys.teku.validator.api.ValidatorApiChannel; import tech.pegasys.teku.validator.api.ValidatorTimingChannel; import tech.pegasys.teku.validator.client.loader.OwnedValidators; @@ -59,18 +62,21 @@ public class ValidatorRegistrator implements ValidatorTimingChannel { private final OwnedValidators ownedValidators; private final ProposerConfigPropertiesProvider validatorRegistrationPropertiesProvider; private final ValidatorRegistrationBatchSender validatorRegistrationBatchSender; + private final ValidatorApiChannel validatorApiChannel; public ValidatorRegistrator( final Spec spec, final TimeProvider timeProvider, final OwnedValidators ownedValidators, final ProposerConfigPropertiesProvider validatorRegistrationPropertiesProvider, - final ValidatorRegistrationBatchSender validatorRegistrationBatchSender) { + final ValidatorRegistrationBatchSender validatorRegistrationBatchSender, + final ValidatorApiChannel validatorApiChannel) { this.spec = spec; this.timeProvider = timeProvider; this.ownedValidators = ownedValidators; this.validatorRegistrationPropertiesProvider = validatorRegistrationPropertiesProvider; this.validatorRegistrationBatchSender = validatorRegistrationBatchSender; + this.validatorApiChannel = validatorApiChannel; } @Override @@ -158,13 +164,13 @@ private void registerValidators() { "Validator registration(s) is still in progress. Will skip sending registration(s)."); return; } - final List activeValidators = ownedValidators.getActiveValidators(); - registerValidators(activeValidators) + final List managedValidators = ownedValidators.getActiveValidators(); + registerValidators(managedValidators) .handleException(VALIDATOR_LOGGER::registeringValidatorsFailed) .always( () -> { registrationInProgress.set(false); - cleanupCache(activeValidators); + cleanupCache(managedValidators); }); } @@ -177,28 +183,46 @@ private SafeFuture registerValidators(final List validators) { .refresh() .thenCompose( __ -> { - final Stream> validatorRegistrationsFutures = - createValidatorRegistrations(validators); - return SafeFuture.collectAllSuccessful(validatorRegistrationsFutures) - .thenCompose(validatorRegistrationBatchSender::sendInBatches); + final SafeFuture> validatorRegistrations = + filterActiveValidators(validators) + .thenCompose(this::createValidatorRegistrations); + return validatorRegistrations.thenCompose( + validatorRegistrationBatchSender::sendInBatches); }); } - private Stream> createValidatorRegistrations( + private SafeFuture> filterActiveValidators(final List validators) { + final Map validatorMap = + validators.stream().collect(Collectors.toMap(Validator::getPublicKey, Function.identity())); + return validatorApiChannel + .getValidatorStatuses(validators.stream().map(Validator::getPublicKey).toList()) + .thenApply( + maybeValidatorStatuses -> + maybeValidatorStatuses.map(Map::entrySet).stream() + .flatMap(Collection::stream) + .filter(statusEntry -> statusEntry.getValue().isActive()) + .map(statusEntry -> Optional.ofNullable(validatorMap.get(statusEntry.getKey()))) + .flatMap(Optional::stream) + .toList()); + } + + private SafeFuture> createValidatorRegistrations( final List validators) { - return validators.stream() - .map( - validator -> - createSignedValidatorRegistration( - validator, - throwable -> { - final String errorMessage = - String.format( - "Exception while creating a validator registration for %s. Creation will be attempted again next epoch.", - validator.getPublicKey()); - LOG.warn(errorMessage, throwable); - })) - .flatMap(Optional::stream); + final Stream> validatorRegistrationsFutures = + validators.stream() + .map( + validator -> + createSignedValidatorRegistration( + validator, + throwable -> { + final String errorMessage = + String.format( + "Exception while creating a validator registration for %s. Creation will be attempted again next epoch.", + validator.getPublicKey()); + LOG.warn(errorMessage, throwable); + })) + .flatMap(Optional::stream); + return SafeFuture.collectAllSuccessful(validatorRegistrationsFutures); } private Optional> createSignedValidatorRegistration( @@ -315,14 +339,14 @@ public boolean registrationNeedsUpdating( || cachedTimestampIsDifferentThanOverride; } - private void cleanupCache(final List activeValidators) { + private void cleanupCache(final List managedValidators) { if (cachedValidatorRegistrations.isEmpty() - || cachedValidatorRegistrations.size() == activeValidators.size()) { + || cachedValidatorRegistrations.size() == managedValidators.size()) { return; } - final Set activeValidatorsPublicKeys = - activeValidators.stream() + final Set managedValidatorsPublicKeys = + managedValidators.stream() .map(Validator::getPublicKey) .collect(Collectors.toCollection(HashSet::new)); @@ -331,10 +355,10 @@ private void cleanupCache(final List activeValidators) { .removeIf( cachedPublicKey -> { final boolean requiresRemoving = - !activeValidatorsPublicKeys.contains(cachedPublicKey); + !managedValidatorsPublicKeys.contains(cachedPublicKey); if (requiresRemoving) { LOG.debug( - "Removing cached registration for {} because validator is no longer active.", + "Removing cached registration for {} because validator is no longer owned.", cachedPublicKey); } return requiresRemoving; diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java index f99451e2e3e..381f7a145ea 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java @@ -15,6 +15,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; @@ -31,10 +32,12 @@ import java.util.Map; import java.util.Optional; import java.util.function.Consumer; +import java.util.function.Function; import java.util.stream.Collectors; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.TestTemplate; import org.mockito.ArgumentCaptor; +import tech.pegasys.teku.api.response.v1.beacon.ValidatorStatus; import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.ethereum.execution.types.Eth1Address; import tech.pegasys.teku.infrastructure.async.SafeFuture; @@ -48,6 +51,7 @@ import tech.pegasys.teku.spec.datastructures.builder.ValidatorRegistration; import tech.pegasys.teku.spec.signatures.Signer; import tech.pegasys.teku.spec.util.DataStructureUtil; +import tech.pegasys.teku.validator.api.ValidatorApiChannel; import tech.pegasys.teku.validator.client.loader.OwnedValidators; @TestSpecContext(milestone = SpecMilestone.BELLATRIX) @@ -58,6 +62,7 @@ class ValidatorRegistratorTest { mock(ProposerConfigPropertiesProvider.class); private final ValidatorRegistrationBatchSender validatorRegistrationBatchSender = mock(ValidatorRegistrationBatchSender.class); + private final ValidatorApiChannel validatorApiChannel = mock(ValidatorApiChannel.class); private final TimeProvider stubTimeProvider = StubTimeProvider.withTimeInSeconds(12); private final Signer signer = mock(Signer.class); @@ -74,6 +79,7 @@ class ValidatorRegistratorTest { private ValidatorRegistrator validatorRegistrator; @BeforeEach + @SuppressWarnings("unchecked") void setUp(SpecContext specContext) { slotsPerEpoch = specContext.getSpec().getGenesisSpecConfig().getSlotsPerEpoch(); dataStructureUtil = specContext.getDataStructureUtil(); @@ -90,7 +96,8 @@ void setUp(SpecContext specContext) { stubTimeProvider, ownedValidators, proposerConfigPropertiesProvider, - validatorRegistrationBatchSender); + validatorRegistrationBatchSender, + validatorApiChannel); when(validatorRegistrationBatchSender.sendInBatches(any())).thenReturn(SafeFuture.COMPLETE); when(proposerConfigPropertiesProvider.isBuilderEnabled(any())).thenReturn(true); @@ -106,6 +113,19 @@ void setUp(SpecContext specContext) { doAnswer(invocation -> SafeFuture.completedFuture(dataStructureUtil.randomSignature())) .when(signer) .signValidatorRegistration(any(ValidatorRegistration.class)); + + // all validators are active + when(validatorApiChannel.getValidatorStatuses(anyList())) + .thenAnswer( + args -> { + final List keys = (List) args.getArguments()[0]; + Map statuses = + keys.stream() + .collect( + Collectors.toMap( + Function.identity(), __ -> ValidatorStatus.active_ongoing)); + return SafeFuture.completedFuture(Optional.of(statuses)); + }); } @TestTemplate @@ -119,7 +139,7 @@ void doesNotRegisterValidators_ifNotReady() { @TestTemplate void doesNotRegisterValidators_ifNotThreeSlotsInTheEpoch() { - setActiveValidators(validator1, validator2, validator3); + setOwnedValidators(validator1, validator2, validator3); // initially validators will be registered anyway since it's the first call runRegistrationFlowForSlot(ZERO); @@ -135,7 +155,7 @@ void doesNotRegisterValidators_ifNotThreeSlotsInTheEpoch() { @TestTemplate void registersValidators_threeSlotsInTheEpoch() { - setActiveValidators(validator1, validator2, validator3); + setOwnedValidators(validator1, validator2, validator3); runRegistrationFlowForEpoch(0); runRegistrationFlowForEpoch(1); @@ -159,7 +179,7 @@ void registersValidators_shouldRegisterWithTimestampOverride() { validator1.getPublicKey())) .thenReturn(Optional.of(timestampOverride)); - setActiveValidators(validator1); + setOwnedValidators(validator1); runRegistrationFlowForEpoch(0); runRegistrationFlowForEpoch(1); @@ -189,7 +209,7 @@ void registersValidators_shouldRegisterWithPublicKeyOverride() { validator2.getPublicKey())) .thenReturn(Optional.of(publicKeyOverride)); - setActiveValidators(validator1, validator2); + setOwnedValidators(validator1, validator2); runRegistrationFlowForEpoch(0); runRegistrationFlowForEpoch(1); @@ -211,15 +231,15 @@ void registersValidators_shouldRegisterWithPublicKeyOverride() { } @TestTemplate - void cleanupsCache_ifValidatorIsNoLongerActive() { - setActiveValidators(validator1, validator2, validator3); + void cleanupsCache_ifValidatorIsNoLongerOwned() { + setOwnedValidators(validator1, validator2, validator3); runRegistrationFlowForEpoch(0); assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(3); // validator1 not active anymore - setActiveValidators(validator2, validator3); + setOwnedValidators(validator2, validator3); runRegistrationFlowForEpoch(1); @@ -233,7 +253,7 @@ void doesNotUseCache_ifRegistrationsNeedUpdating() { final Validator validator5 = new Validator(dataStructureUtil.randomPublicKey(), signer, Optional::empty); - setActiveValidators(validator1, validator2, validator3, validator4, validator5); + setOwnedValidators(validator1, validator2, validator3, validator4, validator5); runRegistrationFlowForEpoch(0); @@ -316,7 +336,7 @@ void doesNotRegisterValidatorsOnPossibleMissedEvents_ifNotReady() { @TestTemplate void registerValidatorsOnPossibleMissedEvents() { - setActiveValidators(validator1, validator2, validator3); + setOwnedValidators(validator1, validator2, validator3); validatorRegistrator.onPossibleMissedEvents(); @@ -343,12 +363,12 @@ void doesNotRegisterNewlyAddedValidators_ifFirstCallHasNotBeenDone() { @TestTemplate void registersNewlyAddedValidators() { - setActiveValidators(validator1); + setOwnedValidators(validator1); runRegistrationFlowForEpoch(0); // new validators are added - setActiveValidators(validator1, validator2, validator3); + setOwnedValidators(validator1, validator2, validator3); validatorRegistrator.onValidatorsAdded(); @@ -365,7 +385,7 @@ void registersNewlyAddedValidators() { @TestTemplate void skipsValidatorRegistrationIfRegistrationNotEnabled() { - setActiveValidators(validator1, validator2, validator3); + setOwnedValidators(validator1, validator2, validator3); // validator registration is disabled for validator2 when(proposerConfigPropertiesProvider.isBuilderEnabled(validator2.getPublicKey())) @@ -381,7 +401,7 @@ void skipsValidatorRegistrationIfRegistrationNotEnabled() { @TestTemplate void retrievesCorrectGasLimitForValidators() { - setActiveValidators(validator1, validator2); + setOwnedValidators(validator1, validator2); final UInt64 validator2GasLimit = UInt64.valueOf(28_000_000); final UInt64 validator1GasLimit = UInt64.valueOf(27_000_000); @@ -414,7 +434,7 @@ void retrievesCorrectGasLimitForValidators() { @TestTemplate void skipsValidatorRegistrationIfFeeRecipientNotSpecified() { - setActiveValidators(validator1, validator2); + setOwnedValidators(validator1, validator2); // no fee recipient provided for validator2 when(proposerConfigPropertiesProvider.getFeeRecipient(validator2.getPublicKey())) @@ -428,7 +448,7 @@ void skipsValidatorRegistrationIfFeeRecipientNotSpecified() { @TestTemplate void registerValidatorsEvenIfOneRegistrationSigningFails() { - setActiveValidators(validator1, validator2, validator3); + setOwnedValidators(validator1, validator2, validator3); when(signer.signValidatorRegistration( argThat( @@ -448,7 +468,72 @@ void registerValidatorsEvenIfOneRegistrationSigningFails() { verifyRegistrations(registrationCalls.get(1), List.of(validator1, validator2, validator3)); } - private void setActiveValidators(final Validator... validators) { + @TestTemplate + @SuppressWarnings("unchecked") + void doesNotRegister_ifValidatorIsNotActive() { + setOwnedValidators(validator1, validator2, validator3); + + // only validator2 is active + when(validatorApiChannel.getValidatorStatuses(anyList())) + .thenAnswer( + args -> { + final List keys = (List) args.getArguments()[0]; + Map statuses = + keys.stream() + .collect( + Collectors.toMap( + Function.identity(), + publicKey -> { + if (publicKey.equals(validator1.getPublicKey())) { + return ValidatorStatus.pending_initialized; + } + if (publicKey.equals(validator3.getPublicKey())) { + return ValidatorStatus.exited_unslashed; + } + return ValidatorStatus.active_ongoing; + })); + return SafeFuture.completedFuture(Optional.of(statuses)); + }); + + runRegistrationFlowForEpoch(0); + + assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(1); + verify(signer, times(1)).signValidatorRegistration(any()); + + // validator1, validator2 are active + when(validatorApiChannel.getValidatorStatuses(anyList())) + .thenAnswer( + args -> { + final List keys = (List) args.getArguments()[0]; + Map statuses = + keys.stream() + .collect( + Collectors.toMap( + Function.identity(), + publicKey -> { + if (publicKey.equals(validator2.getPublicKey())) { + return ValidatorStatus.active_exiting; + } + if (publicKey.equals(validator3.getPublicKey())) { + return ValidatorStatus.exited_unslashed; + } + return ValidatorStatus.active_ongoing; + })); + return SafeFuture.completedFuture(Optional.of(statuses)); + }); + + runRegistrationFlowForEpoch(1); + + assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(2); + verify(signer, times(2)).signValidatorRegistration(any()); + + final List> registrationCalls = captureRegistrationCalls(2); + + verifyRegistrations(registrationCalls.get(0), List.of(validator2)); + verifyRegistrations(registrationCalls.get(1), List.of(validator1, validator2)); + } + + private void setOwnedValidators(final Validator... validators) { final List validatorsAsList = Arrays.stream(validators).collect(Collectors.toList()); when(ownedValidators.getActiveValidators()).thenReturn(validatorsAsList); } From 578b171fa1edd48390bb92da1e1c085f0e289b8a Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Fri, 18 Aug 2023 21:04:20 +0200 Subject: [PATCH 02/18] Check validator statuses in batches --- .../client/ValidatorClientService.java | 9 +- .../ValidatorRegistrationBatchSender.java | 97 ---- .../ValidatorRegistrationSigningService.java | 158 ++++++ .../client/ValidatorRegistrator.java | 244 ++++----- .../ValidatorRegistrationBatchSenderTest.java | 119 ----- ...lidatorRegistrationSigningServiceTest.java | 346 +++++++++++++ .../client/ValidatorRegistratorTest.java | 476 +++++++----------- 7 files changed, 781 insertions(+), 668 deletions(-) delete mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationBatchSender.java create mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningService.java delete mode 100644 validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationBatchSenderTest.java create mode 100644 validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningServiceTest.java diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java index cb9262b8ab6..ce011487ed4 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java @@ -176,13 +176,12 @@ public static ValidatorClientService create( Optional.of( new ValidatorRegistrator( config.getSpec(), - services.getTimeProvider(), validatorLoader.getOwnedValidators(), proposerConfigManager.get(), - new ValidatorRegistrationBatchSender( - validatorConfig.getBuilderRegistrationSendingBatchSize(), - validatorApiChannel), - validatorApiChannel)); + new ValidatorRegistrationSigningService( + proposerConfigManager.get(), services.getTimeProvider()), + validatorApiChannel, + validatorConfig.getBuilderRegistrationSendingBatchSize())); } else { proposerConfigManager = Optional.empty(); } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationBatchSender.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationBatchSender.java deleted file mode 100644 index 2d900254d59..00000000000 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationBatchSender.java +++ /dev/null @@ -1,97 +0,0 @@ -/* - * Copyright Consensys Software Inc., 2022 - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package tech.pegasys.teku.validator.client; - -import static tech.pegasys.teku.infrastructure.logging.ValidatorLogger.VALIDATOR_LOGGER; - -import com.google.common.collect.Lists; -import java.util.Iterator; -import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import tech.pegasys.teku.infrastructure.async.SafeFuture; -import tech.pegasys.teku.infrastructure.ssz.SszList; -import tech.pegasys.teku.infrastructure.ssz.impl.SszUtils; -import tech.pegasys.teku.spec.datastructures.builder.SignedValidatorRegistration; -import tech.pegasys.teku.spec.schemas.ApiSchemas; -import tech.pegasys.teku.validator.api.ValidatorApiChannel; - -public class ValidatorRegistrationBatchSender { - private static final Logger LOG = LogManager.getLogger(); - - private final int batchSize; - private final ValidatorApiChannel validatorApiChannel; - - public ValidatorRegistrationBatchSender( - final int batchSize, final ValidatorApiChannel validatorApiChannel) { - this.batchSize = batchSize; - this.validatorApiChannel = validatorApiChannel; - } - - public SafeFuture sendInBatches( - final List validatorRegistrations) { - if (validatorRegistrations.isEmpty()) { - LOG.debug("No validator(s) registrations required to be sent to the Beacon Node."); - return SafeFuture.COMPLETE; - } - - final List> batchedRegistrations = - Lists.partition(validatorRegistrations, batchSize); - - LOG.debug( - "Going to send {} validator registration(s) to the Beacon Node in {} batch(es)", - validatorRegistrations.size(), - batchedRegistrations.size()); - - final Iterator> batchedRegistrationsIterator = - batchedRegistrations.iterator(); - - final AtomicInteger batchCounter = new AtomicInteger(0); - final AtomicInteger successfullySentRegistrations = new AtomicInteger(0); - - return SafeFuture.asyncDoWhile( - () -> { - if (!batchedRegistrationsIterator.hasNext()) { - return SafeFuture.completedFuture(false); - } - final List batch = batchedRegistrationsIterator.next(); - final int currentBatch = batchCounter.incrementAndGet(); - LOG.debug("Starting to send batch {}/{}", currentBatch, batchedRegistrations.size()); - return sendBatch(batch) - .thenApply( - __ -> { - successfullySentRegistrations.updateAndGet(count -> count + batch.size()); - LOG.debug( - "Batch {}/{}: {} validator(s) registrations were sent to the Beacon Node.", - currentBatch, - batchedRegistrations.size(), - batch.size()); - return true; - }); - }) - .alwaysRun( - () -> - VALIDATOR_LOGGER.validatorRegistrationsSentToTheBuilderNetwork( - successfullySentRegistrations.get(), validatorRegistrations.size())); - } - - private SafeFuture sendBatch( - final List validatorRegistrations) { - final SszList sszValidatorRegistrations = - SszUtils.toSszList( - ApiSchemas.SIGNED_VALIDATOR_REGISTRATIONS_SCHEMA, validatorRegistrations); - return validatorApiChannel.registerValidators(sszValidatorRegistrations); - } -} diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningService.java new file mode 100644 index 00000000000..ab1e4dfc910 --- /dev/null +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningService.java @@ -0,0 +1,158 @@ +/* + * Copyright Consensys Software Inc., 2022 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.validator.client; + +import java.util.Optional; +import java.util.function.Consumer; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import tech.pegasys.teku.bls.BLSPublicKey; +import tech.pegasys.teku.ethereum.execution.types.Eth1Address; +import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.infrastructure.time.TimeProvider; +import tech.pegasys.teku.infrastructure.unsigned.UInt64; +import tech.pegasys.teku.spec.datastructures.builder.SignedValidatorRegistration; +import tech.pegasys.teku.spec.datastructures.builder.ValidatorRegistration; +import tech.pegasys.teku.spec.schemas.ApiSchemas; +import tech.pegasys.teku.spec.signatures.Signer; + +public class ValidatorRegistrationSigningService { + private static final Logger LOG = LogManager.getLogger(); + + private final ProposerConfigPropertiesProvider validatorRegistrationPropertiesProvider; + private final TimeProvider timeProvider; + + public ValidatorRegistrationSigningService( + final ProposerConfigPropertiesProvider validatorRegistrationPropertiesProvider, + final TimeProvider timeProvider) { + this.validatorRegistrationPropertiesProvider = validatorRegistrationPropertiesProvider; + this.timeProvider = timeProvider; + } + + public Optional> createSignedValidatorRegistration( + final Validator validator, + final Optional oldValidatorRegistration, + final Consumer errorHandler) { + return createSignedValidatorRegistration(validator, oldValidatorRegistration) + .map(registrationFuture -> registrationFuture.whenException(errorHandler)); + } + + private Optional> createSignedValidatorRegistration( + final Validator validator, + final Optional oldValidatorRegistration) { + + final BLSPublicKey publicKey = validator.getPublicKey(); + + final boolean builderEnabled = + validatorRegistrationPropertiesProvider.isBuilderEnabled(publicKey); + + if (!builderEnabled) { + LOG.trace("Validator registration is disabled for {}", publicKey); + return Optional.empty(); + } + + final Optional maybeFeeRecipient = + validatorRegistrationPropertiesProvider.getFeeRecipient(publicKey); + + if (maybeFeeRecipient.isEmpty()) { + LOG.debug( + "Couldn't retrieve fee recipient for {}. Will skip registering this validator.", + publicKey); + return Optional.empty(); + } + + final Eth1Address feeRecipient = maybeFeeRecipient.get(); + final UInt64 gasLimit = validatorRegistrationPropertiesProvider.getGasLimit(publicKey); + + final Optional maybeTimestampOverride = + validatorRegistrationPropertiesProvider.getBuilderRegistrationTimestampOverride(publicKey); + final Optional maybePublicKeyOverride = + validatorRegistrationPropertiesProvider.getBuilderRegistrationPublicKeyOverride(publicKey); + + final ValidatorRegistration validatorRegistration = + createValidatorRegistration( + maybePublicKeyOverride.orElse(publicKey), + feeRecipient, + gasLimit, + maybeTimestampOverride.orElse(timeProvider.getTimeInSeconds())); + + return oldValidatorRegistration + .filter( + cachedValidatorRegistration -> { + final boolean needsUpdate = + registrationNeedsUpdating( + cachedValidatorRegistration.getMessage(), + validatorRegistration, + maybeTimestampOverride); + if (needsUpdate) { + LOG.debug( + "The cached registration for {} needs updating. Will create a new one.", + publicKey); + } + return !needsUpdate; + }) + .map(SafeFuture::completedFuture) + .or( + () -> { + final Signer signer = validator.getSigner(); + return Optional.of( + signAndCacheValidatorRegistration(publicKey, validatorRegistration, signer)); + }); + } + + private SafeFuture signAndCacheValidatorRegistration( + final BLSPublicKey cacheKey, + final ValidatorRegistration validatorRegistration, + final Signer signer) { + return signer + .signValidatorRegistration(validatorRegistration) + .thenApply( + signature -> { + final SignedValidatorRegistration signedValidatorRegistration = + ApiSchemas.SIGNED_VALIDATOR_REGISTRATION_SCHEMA.create( + validatorRegistration, signature); + LOG.debug("Validator registration signed for {}", cacheKey); + return signedValidatorRegistration; + }); + } + + private boolean registrationNeedsUpdating( + final ValidatorRegistration cachedValidatorRegistration, + final ValidatorRegistration newValidatorRegistration, + final Optional newMaybeTimestampOverride) { + final boolean cachedTimestampIsDifferentThanOverride = + newMaybeTimestampOverride + .map( + newTimestampOverride -> + !cachedValidatorRegistration.getTimestamp().equals(newTimestampOverride)) + .orElse(false); + return !cachedValidatorRegistration + .getFeeRecipient() + .equals(newValidatorRegistration.getFeeRecipient()) + || !cachedValidatorRegistration.getGasLimit().equals(newValidatorRegistration.getGasLimit()) + || !cachedValidatorRegistration + .getPublicKey() + .equals(newValidatorRegistration.getPublicKey()) + || cachedTimestampIsDifferentThanOverride; + } + + private ValidatorRegistration createValidatorRegistration( + final BLSPublicKey publicKey, + final Eth1Address feeRecipient, + final UInt64 gasLimit, + final UInt64 timestamp) { + return ApiSchemas.VALIDATOR_REGISTRATION_SCHEMA.create( + feeRecipient, gasLimit, timestamp, publicKey); + } +} diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java index 0a12f178288..f0c067b2598 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java @@ -15,33 +15,34 @@ import static tech.pegasys.teku.infrastructure.logging.ValidatorLogger.VALIDATOR_LOGGER; +import com.google.common.collect.Lists; import com.google.common.collect.Maps; import java.util.Collection; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.apache.commons.lang3.tuple.Pair; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.tuweni.bytes.Bytes32; import tech.pegasys.teku.bls.BLSPublicKey; -import tech.pegasys.teku.ethereum.execution.types.Eth1Address; import tech.pegasys.teku.infrastructure.async.SafeFuture; -import tech.pegasys.teku.infrastructure.time.TimeProvider; +import tech.pegasys.teku.infrastructure.ssz.SszList; +import tech.pegasys.teku.infrastructure.ssz.impl.SszUtils; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.config.Constants; import tech.pegasys.teku.spec.datastructures.builder.SignedValidatorRegistration; -import tech.pegasys.teku.spec.datastructures.builder.ValidatorRegistration; import tech.pegasys.teku.spec.schemas.ApiSchemas; -import tech.pegasys.teku.spec.signatures.Signer; import tech.pegasys.teku.validator.api.ValidatorApiChannel; import tech.pegasys.teku.validator.api.ValidatorTimingChannel; import tech.pegasys.teku.validator.client.loader.OwnedValidators; @@ -58,25 +59,25 @@ public class ValidatorRegistrator implements ValidatorTimingChannel { private final AtomicReference lastRunEpoch = new AtomicReference<>(); private final Spec spec; - private final TimeProvider timeProvider; private final OwnedValidators ownedValidators; private final ProposerConfigPropertiesProvider validatorRegistrationPropertiesProvider; - private final ValidatorRegistrationBatchSender validatorRegistrationBatchSender; + private final ValidatorRegistrationSigningService validatorRegistrationSigningService; private final ValidatorApiChannel validatorApiChannel; + private final int batchSize; public ValidatorRegistrator( final Spec spec, - final TimeProvider timeProvider, final OwnedValidators ownedValidators, final ProposerConfigPropertiesProvider validatorRegistrationPropertiesProvider, - final ValidatorRegistrationBatchSender validatorRegistrationBatchSender, - final ValidatorApiChannel validatorApiChannel) { + final ValidatorRegistrationSigningService validatorRegistrationSigningService, + final ValidatorApiChannel validatorApiChannel, + final int batchSize) { this.spec = spec; - this.timeProvider = timeProvider; this.ownedValidators = ownedValidators; this.validatorRegistrationPropertiesProvider = validatorRegistrationPropertiesProvider; - this.validatorRegistrationBatchSender = validatorRegistrationBatchSender; + this.validatorRegistrationSigningService = validatorRegistrationSigningService; this.validatorApiChannel = validatorApiChannel; + this.batchSize = batchSize; } @Override @@ -181,21 +182,64 @@ private SafeFuture registerValidators(final List validators) { return validatorRegistrationPropertiesProvider .refresh() - .thenCompose( - __ -> { - final SafeFuture> validatorRegistrations = - filterActiveValidators(validators) - .thenCompose(this::createValidatorRegistrations); - return validatorRegistrations.thenCompose( - validatorRegistrationBatchSender::sendInBatches); - }); + .thenCompose(__ -> processInBatches(validators)); + } + + private SafeFuture processInBatches(final List validators) { + final List> batchedValidators = Lists.partition(validators, batchSize); + + LOG.debug( + "Going to prepare and send {} validator registration(s) to the Beacon Node in {} batch(es)", + validators.size(), + batchedValidators.size()); + + final Iterator> batchedValidatorsIterator = batchedValidators.iterator(); + + final AtomicInteger batchCounter = new AtomicInteger(0); + final AtomicInteger successfullySentRegistrations = new AtomicInteger(0); + + return SafeFuture.asyncDoWhile( + () -> { + if (!batchedValidatorsIterator.hasNext()) { + return SafeFuture.completedFuture(false); + } + final List batch = batchedValidatorsIterator.next(); + final int currentBatch = batchCounter.incrementAndGet(); + LOG.debug( + "Starting to process validators registration batch {}/{}", + currentBatch, + batchedValidators.size()); + return filterActiveValidators(batch) + .thenCompose(this::createValidatorRegistrations) + .thenCompose(this::sendValidatorRegistrations) + .thenApply( + size -> { + successfullySentRegistrations.updateAndGet(count -> count + size); + LOG.debug( + "Batch {}/{}: {} validator(s) registrations were sent to the Beacon Node out of {} validators.", + currentBatch, + batchedValidators.size(), + size, + batch.size()); + return true; + }); + }) + .alwaysRun( + () -> + VALIDATOR_LOGGER.validatorRegistrationsSentToTheBuilderNetwork( + successfullySentRegistrations.get(), validators.size())); } private SafeFuture> filterActiveValidators(final List validators) { + final Function getKey = + validator -> + validatorRegistrationPropertiesProvider + .getBuilderRegistrationPublicKeyOverride(validator.getPublicKey()) + .orElse(validator.getPublicKey()); final Map validatorMap = - validators.stream().collect(Collectors.toMap(Validator::getPublicKey, Function.identity())); + validators.stream().collect(Collectors.toMap(getKey, Function.identity())); return validatorApiChannel - .getValidatorStatuses(validators.stream().map(Validator::getPublicKey).toList()) + .getValidatorStatuses(validators.stream().map(getKey).toList()) .thenApply( maybeValidatorStatuses -> maybeValidatorStatuses.map(Map::entrySet).stream() @@ -211,132 +255,42 @@ private SafeFuture> createValidatorRegistratio final Stream> validatorRegistrationsFutures = validators.stream() .map( - validator -> - createSignedValidatorRegistration( - validator, - throwable -> { - final String errorMessage = - String.format( - "Exception while creating a validator registration for %s. Creation will be attempted again next epoch.", - validator.getPublicKey()); - LOG.warn(errorMessage, throwable); - })) - .flatMap(Optional::stream); + validator -> { + final Optional> + maybeSignedValidatorRegistration = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator, + Optional.ofNullable( + cachedValidatorRegistrations.get(validator.getPublicKey())), + throwable -> { + final String errorMessage = + String.format( + "Exception while creating a validator registration for %s. Creation will be attempted again next epoch.", + validator.getPublicKey()); + LOG.warn(errorMessage, throwable); + }); + return Pair.of(validator.getPublicKey(), maybeSignedValidatorRegistration); + }) + .filter(pair -> pair.getRight().isPresent()) + .map( + pair -> { + final SafeFuture registrationFuture = + pair.getRight().get(); + return registrationFuture.thenPeek( + registration -> + cachedValidatorRegistrations.put(pair.getLeft(), registration)); + }); return SafeFuture.collectAllSuccessful(validatorRegistrationsFutures); } - private Optional> createSignedValidatorRegistration( - final Validator validator, final Consumer errorHandler) { - return createSignedValidatorRegistration(validator) - .map(registrationFuture -> registrationFuture.whenException(errorHandler)); - } - - private Optional> createSignedValidatorRegistration( - final Validator validator) { - - final BLSPublicKey publicKey = validator.getPublicKey(); - - final boolean builderEnabled = - validatorRegistrationPropertiesProvider.isBuilderEnabled(publicKey); - - if (!builderEnabled) { - LOG.trace("Validator registration is disabled for {}", publicKey); - return Optional.empty(); - } - - final Optional maybeFeeRecipient = - validatorRegistrationPropertiesProvider.getFeeRecipient(publicKey); - - if (maybeFeeRecipient.isEmpty()) { - LOG.debug( - "Couldn't retrieve fee recipient for {}. Will skip registering this validator.", - publicKey); - return Optional.empty(); - } - - final Eth1Address feeRecipient = maybeFeeRecipient.get(); - final UInt64 gasLimit = validatorRegistrationPropertiesProvider.getGasLimit(publicKey); - - final Optional maybeTimestampOverride = - validatorRegistrationPropertiesProvider.getBuilderRegistrationTimestampOverride(publicKey); - final Optional maybePublicKeyOverride = - validatorRegistrationPropertiesProvider.getBuilderRegistrationPublicKeyOverride(publicKey); - - final ValidatorRegistration validatorRegistration = - createValidatorRegistration( - maybePublicKeyOverride.orElse(publicKey), - feeRecipient, - gasLimit, - maybeTimestampOverride.orElse(timeProvider.getTimeInSeconds())); - - return Optional.ofNullable(cachedValidatorRegistrations.get(publicKey)) - .filter( - cachedValidatorRegistration -> { - final boolean needsUpdate = - registrationNeedsUpdating( - cachedValidatorRegistration.getMessage(), - validatorRegistration, - maybeTimestampOverride); - if (needsUpdate) { - LOG.debug( - "The cached registration for {} needs updating. Will create a new one.", - publicKey); - } - return !needsUpdate; - }) - .map(SafeFuture::completedFuture) - .or( - () -> { - final Signer signer = validator.getSigner(); - return Optional.of( - signAndCacheValidatorRegistration(publicKey, validatorRegistration, signer)); - }); - } - - private ValidatorRegistration createValidatorRegistration( - final BLSPublicKey publicKey, - final Eth1Address feeRecipient, - final UInt64 gasLimit, - final UInt64 timestamp) { - return ApiSchemas.VALIDATOR_REGISTRATION_SCHEMA.create( - feeRecipient, gasLimit, timestamp, publicKey); - } - - private SafeFuture signAndCacheValidatorRegistration( - final BLSPublicKey cacheKey, - final ValidatorRegistration validatorRegistration, - final Signer signer) { - return signer - .signValidatorRegistration(validatorRegistration) - .thenApply( - signature -> { - final SignedValidatorRegistration signedValidatorRegistration = - ApiSchemas.SIGNED_VALIDATOR_REGISTRATION_SCHEMA.create( - validatorRegistration, signature); - LOG.debug("Validator registration signed for {}", cacheKey); - cachedValidatorRegistrations.put(cacheKey, signedValidatorRegistration); - return signedValidatorRegistration; - }); - } - - public boolean registrationNeedsUpdating( - final ValidatorRegistration cachedValidatorRegistration, - final ValidatorRegistration newValidatorRegistration, - final Optional newMaybeTimestampOverride) { - final boolean cachedTimestampIsDifferentThanOverride = - newMaybeTimestampOverride - .map( - newTimestampOverride -> - !cachedValidatorRegistration.getTimestamp().equals(newTimestampOverride)) - .orElse(false); - return !cachedValidatorRegistration - .getFeeRecipient() - .equals(newValidatorRegistration.getFeeRecipient()) - || !cachedValidatorRegistration.getGasLimit().equals(newValidatorRegistration.getGasLimit()) - || !cachedValidatorRegistration - .getPublicKey() - .equals(newValidatorRegistration.getPublicKey()) - || cachedTimestampIsDifferentThanOverride; + private SafeFuture sendValidatorRegistrations( + final List validatorRegistrations) { + final SszList sszValidatorRegistrations = + SszUtils.toSszList( + ApiSchemas.SIGNED_VALIDATOR_REGISTRATIONS_SCHEMA, validatorRegistrations); + return validatorApiChannel + .registerValidators(sszValidatorRegistrations) + .thenApply(__ -> validatorRegistrations.size()); } private void cleanupCache(final List managedValidators) { diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationBatchSenderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationBatchSenderTest.java deleted file mode 100644 index 52ec12bd61f..00000000000 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationBatchSenderTest.java +++ /dev/null @@ -1,119 +0,0 @@ -/* - * Copyright Consensys Software Inc., 2022 - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package tech.pegasys.teku.validator.client; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.when; - -import java.util.List; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.TestTemplate; -import org.mockito.ArgumentCaptor; -import tech.pegasys.teku.infrastructure.async.SafeFuture; -import tech.pegasys.teku.infrastructure.ssz.SszList; -import tech.pegasys.teku.spec.SpecMilestone; -import tech.pegasys.teku.spec.TestSpecContext; -import tech.pegasys.teku.spec.TestSpecInvocationContextProvider.SpecContext; -import tech.pegasys.teku.spec.datastructures.builder.SignedValidatorRegistration; -import tech.pegasys.teku.spec.util.DataStructureUtil; -import tech.pegasys.teku.validator.api.ValidatorApiChannel; - -@TestSpecContext(milestone = SpecMilestone.BELLATRIX) -class ValidatorRegistrationBatchSenderTest { - - private static final int BATCH_SIZE = 2; - - private final ValidatorApiChannel validatorApiChannel = mock(ValidatorApiChannel.class); - - private DataStructureUtil dataStructureUtil; - private ValidatorRegistrationBatchSender validatorRegistrationBatchSender; - - @BeforeEach - void setUp(SpecContext specContext) { - dataStructureUtil = specContext.getDataStructureUtil(); - - when(validatorApiChannel.registerValidators(any())).thenReturn(SafeFuture.COMPLETE); - - validatorRegistrationBatchSender = - new ValidatorRegistrationBatchSender(BATCH_SIZE, validatorApiChannel); - } - - @TestTemplate - void noRegistrationsAreSentIfEmpty() { - final SafeFuture result = validatorRegistrationBatchSender.sendInBatches(List.of()); - - assertThat(result).isCompleted(); - - verifyNoInteractions(validatorApiChannel); - } - - @TestTemplate - void sendsRegistrationsInBatches() { - SignedValidatorRegistration registration1 = - dataStructureUtil.randomSignedValidatorRegistration(); - SignedValidatorRegistration registration2 = - dataStructureUtil.randomSignedValidatorRegistration(); - SignedValidatorRegistration registration3 = - dataStructureUtil.randomSignedValidatorRegistration(); - SignedValidatorRegistration registration4 = - dataStructureUtil.randomSignedValidatorRegistration(); - SignedValidatorRegistration registration5 = - dataStructureUtil.randomSignedValidatorRegistration(); - - final SafeFuture result = - validatorRegistrationBatchSender.sendInBatches( - List.of(registration1, registration2, registration3, registration4, registration5)); - - assertThat(result).isCompleted(); - - final List> sentRegistrations = - captureSentRegistrations(3); - - assertThat(sentRegistrations.get(0)).containsExactly(registration1, registration2); - assertThat(sentRegistrations.get(1)).containsExactly(registration3, registration4); - assertThat(sentRegistrations.get(2)).containsExactly(registration5); - } - - @TestTemplate - void stopsToSendBatchesOnFirstFailure() { - when(validatorApiChannel.registerValidators(any())) - .thenReturn(SafeFuture.failedFuture(new IllegalStateException("oopsy"))); - - final SafeFuture result = - validatorRegistrationBatchSender.sendInBatches( - dataStructureUtil.randomSignedValidatorRegistrations(5).asList()); - - assertThat(result).isCompletedExceptionally(); - - final List> sentRegistrations = - captureSentRegistrations(1); - - assertThat(sentRegistrations.get(0)).hasSize(BATCH_SIZE); - } - - private List> captureSentRegistrations(final int times) { - @SuppressWarnings("unchecked") - final ArgumentCaptor> argumentCaptor = - ArgumentCaptor.forClass(SszList.class); - - verify(validatorApiChannel, times(times)).registerValidators(argumentCaptor.capture()); - - return argumentCaptor.getAllValues(); - } -} diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningServiceTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningServiceTest.java new file mode 100644 index 00000000000..9c7be05a9a4 --- /dev/null +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningServiceTest.java @@ -0,0 +1,346 @@ +/* + * Copyright Consensys Software Inc., 2022 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.validator.client; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static tech.pegasys.teku.spec.schemas.ApiSchemas.SIGNED_VALIDATOR_REGISTRATION_SCHEMA; +import static tech.pegasys.teku.spec.schemas.ApiSchemas.VALIDATOR_REGISTRATION_SCHEMA; + +import java.util.Optional; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.TestTemplate; +import tech.pegasys.teku.bls.BLSPublicKey; +import tech.pegasys.teku.ethereum.execution.types.Eth1Address; +import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.infrastructure.time.StubTimeProvider; +import tech.pegasys.teku.infrastructure.time.TimeProvider; +import tech.pegasys.teku.infrastructure.unsigned.UInt64; +import tech.pegasys.teku.spec.SpecMilestone; +import tech.pegasys.teku.spec.TestSpecContext; +import tech.pegasys.teku.spec.TestSpecInvocationContextProvider.SpecContext; +import tech.pegasys.teku.spec.datastructures.builder.SignedValidatorRegistration; +import tech.pegasys.teku.spec.datastructures.builder.ValidatorRegistration; +import tech.pegasys.teku.spec.signatures.Signer; +import tech.pegasys.teku.spec.util.DataStructureUtil; + +@TestSpecContext(milestone = SpecMilestone.BELLATRIX) +class ValidatorRegistrationSigningServiceTest { + + private final TimeProvider stubTimeProvider = StubTimeProvider.withTimeInSeconds(12); + private final ProposerConfigPropertiesProvider proposerConfigPropertiesProvider = + mock(ProposerConfigPropertiesProvider.class); + private final Signer signer = mock(Signer.class); + + private Validator validator; + private Eth1Address feeRecipient; + private UInt64 gasLimit; + private ValidatorRegistrationSigningService validatorRegistrationSigningService; + private DataStructureUtil dataStructureUtil; + + @BeforeEach + void setUp(SpecContext specContext) { + dataStructureUtil = specContext.getDataStructureUtil(); + feeRecipient = dataStructureUtil.randomEth1Address(); + gasLimit = dataStructureUtil.randomUInt64(); + validator = new Validator(dataStructureUtil.randomPublicKey(), signer, Optional::empty); + + when(proposerConfigPropertiesProvider.isBuilderEnabled(any())).thenReturn(true); + + when(proposerConfigPropertiesProvider.isReadyToProvideProperties()).thenReturn(true); + when(proposerConfigPropertiesProvider.getFeeRecipient(any())) + .thenReturn(Optional.of(feeRecipient)); + when(proposerConfigPropertiesProvider.getGasLimit(any())).thenReturn(gasLimit); + + when(proposerConfigPropertiesProvider.refresh()).thenReturn(SafeFuture.COMPLETE); + + // random signature for all signings + doAnswer(invocation -> SafeFuture.completedFuture(dataStructureUtil.randomSignature())) + .when(signer) + .signValidatorRegistration(any(ValidatorRegistration.class)); + + validatorRegistrationSigningService = + new ValidatorRegistrationSigningService(proposerConfigPropertiesProvider, stubTimeProvider); + } + + @TestTemplate + public void whenCreateCalled_thenSigningRegistrationIsReturned() { + final Optional> maybeSignedValidatorRegistration = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator, Optional.empty(), throwable -> {}); + + verify(signer).signValidatorRegistration(any()); + assertThat(maybeSignedValidatorRegistration).isPresent(); + final SafeFuture signedValidatorRegistration = + maybeSignedValidatorRegistration.get(); + assertThat(signedValidatorRegistration) + .isCompletedWithValueMatching( + result -> result.getMessage().getPublicKey().equals(validator.getPublicKey())); + } + + @TestTemplate + public void whenOldRegistrationDoesNotNeedToBeUpdated_thenSignerIsNotCalled() { + final SignedValidatorRegistration oldSignedValidatorRegistration = + createSignedValidatorRegistration(validator.getPublicKey(), feeRecipient); + final Optional> maybeSignedValidatorRegistration = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator, Optional.of(oldSignedValidatorRegistration), throwable -> {}); + + verify(signer, never()).signValidatorRegistration(any()); + assertThat(maybeSignedValidatorRegistration).isPresent(); + final SafeFuture signedValidatorRegistration = + maybeSignedValidatorRegistration.get(); + assertThat(signedValidatorRegistration) + .isCompletedWithValueMatching( + result -> result.getMessage().getPublicKey().equals(validator.getPublicKey())); + } + + @TestTemplate + public void whenOldRegistrationNeedsToBeUpdated_thenSignerIsCalled() { + // Old registration with another fee recipient + final SignedValidatorRegistration oldSignedValidatorRegistration = + createSignedValidatorRegistration( + validator.getPublicKey(), dataStructureUtil.randomEth1Address()); + final Optional> maybeSignedValidatorRegistration = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator, Optional.of(oldSignedValidatorRegistration), throwable -> {}); + + verify(signer).signValidatorRegistration(any()); + assertThat(maybeSignedValidatorRegistration).isPresent(); + final SafeFuture signedValidatorRegistration = + maybeSignedValidatorRegistration.get(); + assertThat(signedValidatorRegistration) + .isCompletedWithValueMatching( + result -> + result.getMessage().getPublicKey().equals(validator.getPublicKey()) + && result.getMessage().getFeeRecipient().equals(feeRecipient)); + } + + @TestTemplate + public void whenSigningIsCompletedExceptionally_thenExceptionIsPropagated() { + when(signer.signValidatorRegistration(any())) + .thenReturn(SafeFuture.failedFuture(new RuntimeException("oops"))); + final Optional> maybeSignedValidatorRegistration = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator, Optional.empty(), throwable -> {}); + + verify(signer).signValidatorRegistration(any()); + assertThat(maybeSignedValidatorRegistration).isPresent(); + final SafeFuture signedValidatorRegistration = + maybeSignedValidatorRegistration.get(); + assertThat(signedValidatorRegistration).isCompletedExceptionally(); + } + + @TestTemplate + void shouldRegisterWithTimestampOverride() { + final UInt64 timestampOverride = dataStructureUtil.randomUInt64(); + + when(proposerConfigPropertiesProvider.getBuilderRegistrationTimestampOverride( + validator.getPublicKey())) + .thenReturn(Optional.of(timestampOverride)); + + final Optional> maybeSignedValidatorRegistration = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator, Optional.empty(), throwable -> {}); + verify(signer).signValidatorRegistration(any()); + assertThat(maybeSignedValidatorRegistration).isPresent(); + final SafeFuture signedValidatorRegistration = + maybeSignedValidatorRegistration.get(); + assertThat(signedValidatorRegistration) + .isCompletedWithValueMatching( + result -> + result.getMessage().getPublicKey().equals(validator.getPublicKey()) + && result.getMessage().getTimestamp().equals(timestampOverride)); + } + + @TestTemplate + void shouldRegisterWithPublicKeyOverride() { + final BLSPublicKey publicKeyOverride = dataStructureUtil.randomPublicKey(); + when(proposerConfigPropertiesProvider.getBuilderRegistrationPublicKeyOverride( + validator.getPublicKey())) + .thenReturn(Optional.of(publicKeyOverride)); + + final Optional> maybeSignedValidatorRegistration = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator, Optional.empty(), throwable -> {}); + verify(signer).signValidatorRegistration(any()); + assertThat(maybeSignedValidatorRegistration).isPresent(); + final SafeFuture signedValidatorRegistration = + maybeSignedValidatorRegistration.get(); + assertThat(signedValidatorRegistration) + .isCompletedWithValueMatching( + result -> result.getMessage().getPublicKey().equals(publicKeyOverride)); + } + + @TestTemplate + void setsCorrectGasLimitForValidators() { + final Validator validator2 = + new Validator(dataStructureUtil.randomPublicKey(), signer, Optional::empty); + final UInt64 validator2GasLimit = UInt64.valueOf(27_000_000); + + // validator2 will have custom gas limit + when(proposerConfigPropertiesProvider.getGasLimit(validator2.getPublicKey())) + .thenReturn(validator2GasLimit); + + // Verify validator + final Optional> maybeSignedValidatorRegistration = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator, Optional.empty(), throwable -> {}); + verify(signer).signValidatorRegistration(any()); + assertThat(maybeSignedValidatorRegistration).isPresent(); + final SafeFuture signedValidatorRegistration = + maybeSignedValidatorRegistration.get(); + assertThat(signedValidatorRegistration) + .isCompletedWithValueMatching( + result -> + result.getMessage().getPublicKey().equals(validator.getPublicKey()) + && result.getMessage().getGasLimit().equals(gasLimit)); + + // Verify validator2 + final Optional> maybeSignedValidatorRegistration2 = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator2, Optional.empty(), throwable -> {}); + verify(signer, times(2)).signValidatorRegistration(any()); + assertThat(maybeSignedValidatorRegistration2).isPresent(); + final SafeFuture signedValidatorRegistration2 = + maybeSignedValidatorRegistration2.get(); + assertThat(signedValidatorRegistration2) + .isCompletedWithValueMatching( + result -> + result.getMessage().getPublicKey().equals(validator2.getPublicKey()) + && result.getMessage().getGasLimit().equals(validator2GasLimit)); + } + + @TestTemplate + void setsCorrectTimeStampOverrideForValidators() { + final Validator validator2 = + new Validator(dataStructureUtil.randomPublicKey(), signer, Optional::empty); + final UInt64 validator2Timestamp = UInt64.valueOf(1_000_000); + + // validator2 will have custom gas limit + when(proposerConfigPropertiesProvider.getBuilderRegistrationTimestampOverride( + validator2.getPublicKey())) + .thenReturn(Optional.of(validator2Timestamp)); + + // Verify validator + final Optional> maybeSignedValidatorRegistration = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator, Optional.empty(), throwable -> {}); + verify(signer).signValidatorRegistration(any()); + assertThat(maybeSignedValidatorRegistration).isPresent(); + final SafeFuture signedValidatorRegistration = + maybeSignedValidatorRegistration.get(); + assertThat(signedValidatorRegistration) + .isCompletedWithValueMatching( + result -> + result.getMessage().getPublicKey().equals(validator.getPublicKey()) + && result + .getMessage() + .getTimestamp() + .equals(stubTimeProvider.getTimeInSeconds())); + + // Verify validator2 + final Optional> maybeSignedValidatorRegistration2 = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator2, Optional.empty(), throwable -> {}); + verify(signer, times(2)).signValidatorRegistration(any()); + assertThat(maybeSignedValidatorRegistration2).isPresent(); + final SafeFuture signedValidatorRegistration2 = + maybeSignedValidatorRegistration2.get(); + assertThat(signedValidatorRegistration2) + .isCompletedWithValueMatching( + result -> + result.getMessage().getPublicKey().equals(validator2.getPublicKey()) + && result.getMessage().getTimestamp().equals(validator2Timestamp)); + } + + @TestTemplate + void setsCorrectFeeRecipientForValidators() { + final Validator validator2 = + new Validator(dataStructureUtil.randomPublicKey(), signer, Optional::empty); + final Eth1Address validator2FeeRecipient = dataStructureUtil.randomEth1Address(); + + // validator2 will have custom gas limit + when(proposerConfigPropertiesProvider.getFeeRecipient(validator2.getPublicKey())) + .thenReturn(Optional.of(validator2FeeRecipient)); + + // Verify validator + final Optional> maybeSignedValidatorRegistration = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator, Optional.empty(), throwable -> {}); + verify(signer).signValidatorRegistration(any()); + assertThat(maybeSignedValidatorRegistration).isPresent(); + final SafeFuture signedValidatorRegistration = + maybeSignedValidatorRegistration.get(); + assertThat(signedValidatorRegistration) + .isCompletedWithValueMatching( + result -> + result.getMessage().getPublicKey().equals(validator.getPublicKey()) + && result.getMessage().getFeeRecipient().equals(feeRecipient)); + + // Verify validator2 + final Optional> maybeSignedValidatorRegistration2 = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator2, Optional.empty(), throwable -> {}); + verify(signer, times(2)).signValidatorRegistration(any()); + assertThat(maybeSignedValidatorRegistration2).isPresent(); + final SafeFuture signedValidatorRegistration2 = + maybeSignedValidatorRegistration2.get(); + assertThat(signedValidatorRegistration2) + .isCompletedWithValueMatching( + result -> + result.getMessage().getPublicKey().equals(validator2.getPublicKey()) + && result.getMessage().getFeeRecipient().equals(validator2FeeRecipient)); + } + + @TestTemplate + void doesNotCreateWhenFeeRecipientNotSpecified() { + // no fee recipient provided + when(proposerConfigPropertiesProvider.getFeeRecipient(validator.getPublicKey())) + .thenReturn(Optional.empty()); + + final Optional> maybeSignedValidatorRegistration = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator, Optional.empty(), throwable -> {}); + verify(signer, never()).signValidatorRegistration(any()); + assertThat(maybeSignedValidatorRegistration).isEmpty(); + } + + @TestTemplate + void doesNotCreateWhenValidatorRegistrationNotEnabled() { + // validator registration is disabled for validator2 + when(proposerConfigPropertiesProvider.isBuilderEnabled(validator.getPublicKey())) + .thenReturn(false); + + final Optional> maybeSignedValidatorRegistration = + validatorRegistrationSigningService.createSignedValidatorRegistration( + validator, Optional.empty(), throwable -> {}); + verify(signer, never()).signValidatorRegistration(any()); + assertThat(maybeSignedValidatorRegistration).isEmpty(); + } + + private SignedValidatorRegistration createSignedValidatorRegistration( + final BLSPublicKey publicKey, final Eth1Address customFeeRecipient) { + return SIGNED_VALIDATOR_REGISTRATION_SCHEMA.create( + VALIDATOR_REGISTRATION_SCHEMA.create( + customFeeRecipient, gasLimit, stubTimeProvider.getTimeInSeconds(), publicKey), + dataStructureUtil.randomSignature()); + } +} diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java index 381f7a145ea..dcb8f99b5e3 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java @@ -16,22 +16,23 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; -import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import static tech.pegasys.teku.infrastructure.unsigned.UInt64.ONE; import static tech.pegasys.teku.infrastructure.unsigned.UInt64.ZERO; +import static tech.pegasys.teku.spec.schemas.ApiSchemas.SIGNED_VALIDATOR_REGISTRATION_SCHEMA; +import static tech.pegasys.teku.spec.schemas.ApiSchemas.VALIDATOR_REGISTRATION_SCHEMA; import java.util.Arrays; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Collectors; import org.junit.jupiter.api.BeforeEach; @@ -41,8 +42,7 @@ import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.ethereum.execution.types.Eth1Address; import tech.pegasys.teku.infrastructure.async.SafeFuture; -import tech.pegasys.teku.infrastructure.time.StubTimeProvider; -import tech.pegasys.teku.infrastructure.time.TimeProvider; +import tech.pegasys.teku.infrastructure.ssz.SszList; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.SpecMilestone; import tech.pegasys.teku.spec.TestSpecContext; @@ -57,15 +57,18 @@ @TestSpecContext(milestone = SpecMilestone.BELLATRIX) class ValidatorRegistratorTest { + private static final int BATCH_SIZE = 100; + private static final UInt64 TIMESTAMP = ONE; + private final OwnedValidators ownedValidators = mock(OwnedValidators.class); private final ProposerConfigPropertiesProvider proposerConfigPropertiesProvider = mock(ProposerConfigPropertiesProvider.class); - private final ValidatorRegistrationBatchSender validatorRegistrationBatchSender = - mock(ValidatorRegistrationBatchSender.class); + private final ValidatorRegistrationSigningService validatorRegistrationSigningService = + mock(ValidatorRegistrationSigningService.class); private final ValidatorApiChannel validatorApiChannel = mock(ValidatorApiChannel.class); - private final TimeProvider stubTimeProvider = StubTimeProvider.withTimeInSeconds(12); private final Signer signer = mock(Signer.class); + private SpecContext specContext; private DataStructureUtil dataStructureUtil; private int slotsPerEpoch; @@ -81,6 +84,7 @@ class ValidatorRegistratorTest { @BeforeEach @SuppressWarnings("unchecked") void setUp(SpecContext specContext) { + this.specContext = specContext; slotsPerEpoch = specContext.getSpec().getGenesisSpecConfig().getSlotsPerEpoch(); dataStructureUtil = specContext.getDataStructureUtil(); validator1 = new Validator(dataStructureUtil.randomPublicKey(), signer, Optional::empty); @@ -93,20 +97,22 @@ void setUp(SpecContext specContext) { validatorRegistrator = new ValidatorRegistrator( specContext.getSpec(), - stubTimeProvider, ownedValidators, proposerConfigPropertiesProvider, - validatorRegistrationBatchSender, - validatorApiChannel); - when(validatorRegistrationBatchSender.sendInBatches(any())).thenReturn(SafeFuture.COMPLETE); + validatorRegistrationSigningService, + validatorApiChannel, + BATCH_SIZE); - when(proposerConfigPropertiesProvider.isBuilderEnabled(any())).thenReturn(true); + when(validatorRegistrationSigningService.createSignedValidatorRegistration(any(), any(), any())) + .thenAnswer( + args -> { + final Validator validator = (Validator) args.getArguments()[0]; + return Optional.of( + SafeFuture.completedFuture( + createSignedValidatorRegistration(validator.getPublicKey()))); + }); when(proposerConfigPropertiesProvider.isReadyToProvideProperties()).thenReturn(true); - when(proposerConfigPropertiesProvider.getFeeRecipient(any())) - .thenReturn(Optional.of(eth1Address)); - when(proposerConfigPropertiesProvider.getGasLimit(any())).thenReturn(gasLimit); - when(proposerConfigPropertiesProvider.refresh()).thenReturn(SafeFuture.COMPLETE); // random signature for all signings @@ -126,6 +132,9 @@ void setUp(SpecContext specContext) { Function.identity(), __ -> ValidatorStatus.active_ongoing)); return SafeFuture.completedFuture(Optional.of(statuses)); }); + + // registration is successful by default + when(validatorApiChannel.registerValidators(any())).thenReturn(SafeFuture.COMPLETE); } @TestTemplate @@ -134,7 +143,8 @@ void doesNotRegisterValidators_ifNotReady() { runRegistrationFlowForEpoch(0); - verifyNoInteractions(ownedValidators, validatorRegistrationBatchSender, signer); + verifyNoInteractions( + ownedValidators, validatorApiChannel, validatorRegistrationSigningService, signer); } @TestTemplate @@ -144,90 +154,14 @@ void doesNotRegisterValidators_ifNotThreeSlotsInTheEpoch() { // initially validators will be registered anyway since it's the first call runRegistrationFlowForSlot(ZERO); - verify(validatorRegistrationBatchSender).sendInBatches(any()); + verify(validatorApiChannel).getValidatorStatuses(any()); + verify(validatorApiChannel).registerValidators(any()); // after the initial call, registration should not occur if not three slots in the epoch runRegistrationFlowForSlot( UInt64.valueOf(slotsPerEpoch).plus(UInt64.valueOf(3))); // fourth slot in the epoch - verifyNoMoreInteractions(validatorRegistrationBatchSender); - } - - @TestTemplate - void registersValidators_threeSlotsInTheEpoch() { - setOwnedValidators(validator1, validator2, validator3); - - runRegistrationFlowForEpoch(0); - runRegistrationFlowForEpoch(1); - - final List> registrationCalls = captureRegistrationCalls(2); - - registrationCalls.forEach( - registrationCall -> - verifyRegistrations(registrationCall, List.of(validator1, validator2, validator3))); - - // signer will be called in total 3 times, since from the 2nd run the registrations will - // be cached - verify(signer, times(3)).signValidatorRegistration(any()); - } - - @TestTemplate - void registersValidators_shouldRegisterWithTimestampOverride() { - final UInt64 timestampOverride = dataStructureUtil.randomUInt64(); - - when(proposerConfigPropertiesProvider.getBuilderRegistrationTimestampOverride( - validator1.getPublicKey())) - .thenReturn(Optional.of(timestampOverride)); - - setOwnedValidators(validator1); - - runRegistrationFlowForEpoch(0); - runRegistrationFlowForEpoch(1); - - final List> registrationCalls = captureRegistrationCalls(2); - - registrationCalls.forEach( - registrationCall -> - verifyRegistrations( - registrationCall, - List.of(validator1), - Optional.of( - validatorRegistration -> - assertThat(validatorRegistration.getTimestamp()) - .isEqualTo(timestampOverride)))); - - verify(signer, times(1)).signValidatorRegistration(any()); - } - - @TestTemplate - void registersValidators_shouldRegisterWithPublicKeyOverride() { - final BLSPublicKey publicKeyOverride = dataStructureUtil.randomPublicKey(); - when(proposerConfigPropertiesProvider.getBuilderRegistrationPublicKeyOverride( - validator1.getPublicKey())) - .thenReturn(Optional.of(publicKeyOverride)); - when(proposerConfigPropertiesProvider.getBuilderRegistrationPublicKeyOverride( - validator2.getPublicKey())) - .thenReturn(Optional.of(publicKeyOverride)); - - setOwnedValidators(validator1, validator2); - - runRegistrationFlowForEpoch(0); - runRegistrationFlowForEpoch(1); - - final List> registrationCalls = captureRegistrationCalls(2); - - registrationCalls.forEach( - registrationCall -> - verifyRegistrations( - registrationCall, - List.of(validator1, validator2), - Map.of( - validator1.getPublicKey(), - publicKeyOverride, - validator2.getPublicKey(), - publicKeyOverride))); - - verify(signer, times(2)).signValidatorRegistration(any()); + verifyNoMoreInteractions(validatorApiChannel); } @TestTemplate @@ -246,92 +180,13 @@ void cleanupsCache_ifValidatorIsNoLongerOwned() { assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(2); } - @TestTemplate - void doesNotUseCache_ifRegistrationsNeedUpdating() { - final Validator validator4 = - new Validator(dataStructureUtil.randomPublicKey(), signer, Optional::empty); - final Validator validator5 = - new Validator(dataStructureUtil.randomPublicKey(), signer, Optional::empty); - - setOwnedValidators(validator1, validator2, validator3, validator4, validator5); - - runRegistrationFlowForEpoch(0); - - final Eth1Address otherEth1Address = dataStructureUtil.randomEth1Address(); - final UInt64 otherGasLimit = dataStructureUtil.randomUInt64(); - final BLSPublicKey otherPublicKey = dataStructureUtil.randomPublicKey(); - final UInt64 otherTimestamp = dataStructureUtil.randomUInt64(); - - // fee recipient changed for validator2 - when(proposerConfigPropertiesProvider.getFeeRecipient(validator2.getPublicKey())) - .thenReturn(Optional.of(otherEth1Address)); - - // gas limit changed for validator3 - when(proposerConfigPropertiesProvider.getGasLimit(validator3.getPublicKey())) - .thenReturn(otherGasLimit); - - // public key overwritten for validator4 - when(proposerConfigPropertiesProvider.getBuilderRegistrationPublicKeyOverride( - validator4.getPublicKey())) - .thenReturn(Optional.of(otherPublicKey)); - - // timestamp overwritten for validator5 - when(proposerConfigPropertiesProvider.getBuilderRegistrationTimestampOverride( - validator5.getPublicKey())) - .thenReturn(Optional.of(otherTimestamp)); - - runRegistrationFlowForEpoch(1); - - final List> registrationCalls = captureRegistrationCalls(2); - - // first call should use the default fee recipient, gas limit and public key - verifyRegistrations( - registrationCalls.get(0), - List.of(validator1, validator2, validator3, validator4, validator5)); - - final Consumer updatedRegistrationsRequirements = - (validatorRegistration) -> { - final BLSPublicKey publicKey = validatorRegistration.getPublicKey(); - final Eth1Address feeRecipient = validatorRegistration.getFeeRecipient(); - final UInt64 gasLimit = validatorRegistration.getGasLimit(); - final UInt64 timestamp = validatorRegistration.getTimestamp(); - - if (publicKey.equals(validator1.getPublicKey()) || publicKey.equals(otherPublicKey)) { - assertThat(feeRecipient).isEqualTo(eth1Address); - assertThat(gasLimit).isEqualTo(this.gasLimit); - } - if (publicKey.equals(validator2.getPublicKey())) { - assertThat(feeRecipient).isEqualTo(otherEth1Address); - assertThat(gasLimit).isEqualTo(this.gasLimit); - } - if (publicKey.equals(validator3.getPublicKey())) { - assertThat(feeRecipient).isEqualTo(eth1Address); - assertThat(gasLimit).isEqualTo(otherGasLimit); - } - if (publicKey.equals(validator5.getPublicKey())) { - assertThat(feeRecipient).isEqualTo(eth1Address); - assertThat(gasLimit).isEqualTo(this.gasLimit); - assertThat(timestamp).isEqualTo(otherTimestamp); - } - }; - - // second call should use the changed fee recipient and gas limit, public key and timestamp - verifyRegistrations( - registrationCalls.get(1), - List.of(validator1, validator2, validator3, validator4, validator5), - Optional.of(updatedRegistrationsRequirements), - Map.of(validator4.getPublicKey(), otherPublicKey)); - - verify(signer, times(9)).signValidatorRegistration(any()); - } - @TestTemplate void doesNotRegisterValidatorsOnPossibleMissedEvents_ifNotReady() { when(proposerConfigPropertiesProvider.isReadyToProvideProperties()).thenReturn(false); validatorRegistrator.onPossibleMissedEvents(); - - verifyNoInteractions(ownedValidators, validatorRegistrationBatchSender, signer); + verifyNoInteractions( + ownedValidators, validatorRegistrationSigningService, validatorApiChannel, signer); } @TestTemplate @@ -351,14 +206,16 @@ void doesNotRegisterNewlyAddedValidators_ifNotReady() { validatorRegistrator.onValidatorsAdded(); - verifyNoInteractions(ownedValidators, validatorRegistrationBatchSender, signer); + verifyNoInteractions( + ownedValidators, validatorRegistrationSigningService, validatorApiChannel, signer); } @TestTemplate void doesNotRegisterNewlyAddedValidators_ifFirstCallHasNotBeenDone() { validatorRegistrator.onValidatorsAdded(); - verifyNoInteractions(ownedValidators, validatorRegistrationBatchSender, signer); + verifyNoInteractions( + ownedValidators, validatorRegistrationSigningService, validatorApiChannel, signer); } @TestTemplate @@ -383,83 +240,33 @@ void registersNewlyAddedValidators() { verifyRegistrations(registrationCalls.get(1), List.of(validator2, validator3)); } - @TestTemplate - void skipsValidatorRegistrationIfRegistrationNotEnabled() { - setOwnedValidators(validator1, validator2, validator3); - - // validator registration is disabled for validator2 - when(proposerConfigPropertiesProvider.isBuilderEnabled(validator2.getPublicKey())) - .thenReturn(false); - when(proposerConfigPropertiesProvider.isBuilderEnabled(validator3.getPublicKey())) - .thenReturn(false); - - runRegistrationFlowForEpoch(0); - - final List registrationCalls = captureRegistrationCall(); - verifyRegistrations(registrationCalls, List.of(validator1)); - } - - @TestTemplate - void retrievesCorrectGasLimitForValidators() { - setOwnedValidators(validator1, validator2); - - final UInt64 validator2GasLimit = UInt64.valueOf(28_000_000); - final UInt64 validator1GasLimit = UInt64.valueOf(27_000_000); - - when(proposerConfigPropertiesProvider.getGasLimit(validator1.getPublicKey())) - .thenReturn(validator1GasLimit); - - // validator2 will have custom gas limit - when(proposerConfigPropertiesProvider.getGasLimit(validator2.getPublicKey())) - .thenReturn(validator2GasLimit); - runRegistrationFlowForEpoch(0); - - final List registrationCalls = captureRegistrationCall(); - - final Consumer gasLimitRequirements = - (validatorRegistration) -> { - BLSPublicKey publicKey = validatorRegistration.getPublicKey(); - UInt64 gasLimit = validatorRegistration.getGasLimit(); - if (publicKey.equals(validator1.getPublicKey())) { - assertThat(gasLimit).isEqualTo(validator1GasLimit); - } - if (publicKey.equals(validator2.getPublicKey())) { - assertThat(gasLimit).isEqualTo(validator2GasLimit); - } - }; - - verifyRegistrations( - registrationCalls, List.of(validator1, validator2), Optional.of(gasLimitRequirements)); - } - - @TestTemplate - void skipsValidatorRegistrationIfFeeRecipientNotSpecified() { - setOwnedValidators(validator1, validator2); - - // no fee recipient provided for validator2 - when(proposerConfigPropertiesProvider.getFeeRecipient(validator2.getPublicKey())) - .thenReturn(Optional.empty()); - - runRegistrationFlowForEpoch(0); - - final List registrationCalls = captureRegistrationCall(); - verifyRegistrations(registrationCalls, List.of(validator1)); - } - @TestTemplate void registerValidatorsEvenIfOneRegistrationSigningFails() { setOwnedValidators(validator1, validator2, validator3); - when(signer.signValidatorRegistration( - argThat( - validatorRegistration -> - validatorRegistration.getPublicKey().equals(validator2.getPublicKey())))) - // signing initially fails for validator2 - .thenReturn(SafeFuture.failedFuture(new IllegalStateException("oopsy"))) - // then it succeeds - .thenReturn(SafeFuture.completedFuture(dataStructureUtil.randomSignature())); - + reset(validatorRegistrationSigningService); + when(validatorRegistrationSigningService.createSignedValidatorRegistration(any(), any(), any())) + .thenAnswer( + args -> { + final Validator validator = (Validator) args.getArguments()[0]; + if (validator.equals(validator2)) { + return Optional.of(SafeFuture.failedFuture(new IllegalStateException("oopsy"))); + } + return Optional.of( + SafeFuture.completedFuture( + createSignedValidatorRegistration(validator.getPublicKey()))); + }); runRegistrationFlowForEpoch(0); + + reset(validatorRegistrationSigningService); + when(validatorRegistrationSigningService.createSignedValidatorRegistration(any(), any(), any())) + .thenAnswer( + args -> { + final Validator validator = (Validator) args.getArguments()[0]; + return Optional.of( + SafeFuture.completedFuture( + createSignedValidatorRegistration(validator.getPublicKey()))); + }); runRegistrationFlowForEpoch(1); final List> registrationCalls = captureRegistrationCalls(2); @@ -498,7 +305,6 @@ void doesNotRegister_ifValidatorIsNotActive() { runRegistrationFlowForEpoch(0); assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(1); - verify(signer, times(1)).signValidatorRegistration(any()); // validator1, validator2 are active when(validatorApiChannel.getValidatorStatuses(anyList())) @@ -525,7 +331,6 @@ void doesNotRegister_ifValidatorIsNotActive() { runRegistrationFlowForEpoch(1); assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(2); - verify(signer, times(2)).signValidatorRegistration(any()); final List> registrationCalls = captureRegistrationCalls(2); @@ -533,6 +338,106 @@ void doesNotRegister_ifValidatorIsNotActive() { verifyRegistrations(registrationCalls.get(1), List.of(validator1, validator2)); } + @TestTemplate + void doesNotRegister_ifValidatorStatusNotFound() { + setOwnedValidators(validator1, validator2, validator3); + // validator2 information is only available + when(validatorApiChannel.getValidatorStatuses(anyList())) + .thenReturn( + SafeFuture.completedFuture( + Optional.of(Map.of(validator2.getPublicKey(), ValidatorStatus.active_ongoing)))); + + runRegistrationFlowForEpoch(0); + + assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(1); + final List> registrationCalls = captureRegistrationCalls(1); + verifyRegistrations(registrationCalls.get(0), List.of(validator2)); + } + + @TestTemplate + void noRegistrationsAreSentIfEmpty() { + setOwnedValidators(); + + runRegistrationFlowForSlot(ZERO); + + verifyNoInteractions(validatorApiChannel, signer, validatorRegistrationSigningService); + } + + @TestTemplate + void checksStatusesAndSendsRegistrationsInBatches() { + validatorRegistrator = + new ValidatorRegistrator( + specContext.getSpec(), + ownedValidators, + proposerConfigPropertiesProvider, + validatorRegistrationSigningService, + validatorApiChannel, + 2); + setOwnedValidators(validator1, validator2, validator3); + + runRegistrationFlowForEpoch(0); + + assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(3); + verify(validatorApiChannel) + .getValidatorStatuses(List.of(validator1.getPublicKey(), validator2.getPublicKey())); + verify(validatorApiChannel).getValidatorStatuses(List.of(validator3.getPublicKey())); + + final List> registrationCalls = captureRegistrationCalls(2); + + verifyRegistrations(registrationCalls.get(0), List.of(validator1, validator2)); + verifyRegistrations(registrationCalls.get(1), List.of(validator3)); + } + + @TestTemplate + void stopsToSendBatchesOnFirstFailure() { + when(validatorApiChannel.registerValidators(any())) + .thenReturn(SafeFuture.failedFuture(new IllegalStateException("oopsy"))); + + validatorRegistrator = + new ValidatorRegistrator( + specContext.getSpec(), + ownedValidators, + proposerConfigPropertiesProvider, + validatorRegistrationSigningService, + validatorApiChannel, + 2); + setOwnedValidators(validator1, validator2, validator3); + + runRegistrationFlowForEpoch(0); + + // caching is after signing so still performed for first batch only + assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(2); + // 1 time only, second batch not performed + verify(validatorApiChannel).getValidatorStatuses(anyList()); + verify(validatorApiChannel).registerValidators(any()); + + final List> registrationCalls = captureRegistrationCalls(1); + + verifyRegistrations(registrationCalls.get(0), List.of(validator1, validator2)); + } + + @TestTemplate + void checksValidatorStatusWithPublicKeyOverride() { + final BLSPublicKey validator2KeyOverride = dataStructureUtil.randomPublicKey(); + when(proposerConfigPropertiesProvider.getBuilderRegistrationPublicKeyOverride( + validator2.getPublicKey())) + .thenReturn(Optional.of(validator2KeyOverride)); + setOwnedValidators(validator1, validator2, validator3); + + runRegistrationFlowForEpoch(0); + + assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(3); + // 1 time only, second batch not performed + verify(validatorApiChannel) + .getValidatorStatuses( + List.of(validator1.getPublicKey(), validator2KeyOverride, validator3.getPublicKey())); + verify(validatorApiChannel).registerValidators(any()); + + final List> registrationCalls = captureRegistrationCalls(1); + + verifyRegistrations(registrationCalls.get(0), List.of(validator1, validator2, validator3)); + } + private void setOwnedValidators(final Validator... validators) { final List validatorsAsList = Arrays.stream(validators).collect(Collectors.toList()); when(ownedValidators.getActiveValidators()).thenReturn(validatorsAsList); @@ -554,48 +459,17 @@ private List captureRegistrationCall() { private List> captureRegistrationCalls(final int times) { @SuppressWarnings("unchecked") - final ArgumentCaptor> argumentCaptor = - ArgumentCaptor.forClass(List.class); + final ArgumentCaptor> argumentCaptor = + ArgumentCaptor.forClass(SszList.class); - verify(validatorRegistrationBatchSender, times(times)).sendInBatches(argumentCaptor.capture()); + verify(validatorApiChannel, times(times)).registerValidators(argumentCaptor.capture()); - return argumentCaptor.getAllValues(); + return argumentCaptor.getAllValues().stream().map(SszList::asList).toList(); } private void verifyRegistrations( final List validatorRegistrations, final List expectedRegisteredValidators) { - verifyRegistrations( - validatorRegistrations, expectedRegisteredValidators, Optional.empty(), new HashMap<>()); - } - - private void verifyRegistrations( - final List validatorRegistrations, - final List expectedRegisteredValidators, - final Map expectedPublicKeyOverrides) { - verifyRegistrations( - validatorRegistrations, - expectedRegisteredValidators, - Optional.empty(), - expectedPublicKeyOverrides); - } - - private void verifyRegistrations( - final List validatorRegistrations, - final List expectedRegisteredValidators, - final Optional> alternativeRegistrationRequirements) { - verifyRegistrations( - validatorRegistrations, - expectedRegisteredValidators, - alternativeRegistrationRequirements, - new HashMap<>()); - } - - private void verifyRegistrations( - final List validatorRegistrations, - final List expectedRegisteredValidators, - final Optional> alternativeRegistrationRequirements, - final Map expectedPublicKeyOverrides) { assertThat(validatorRegistrations) .hasSize(expectedRegisteredValidators.size()) @@ -603,23 +477,21 @@ private void verifyRegistrations( .map(SignedValidatorRegistration::getMessage) .allSatisfy( registration -> { - if (alternativeRegistrationRequirements.isPresent()) { - alternativeRegistrationRequirements.get().accept(registration); - } else { - assertThat(registration.getFeeRecipient()).isEqualTo(eth1Address); - assertThat(registration.getTimestamp()) - .isEqualTo(stubTimeProvider.getTimeInSeconds()); - assertThat(registration.getGasLimit()).isEqualTo(gasLimit); - } + assertThat(registration.getFeeRecipient()).isEqualTo(eth1Address); + assertThat(registration.getTimestamp()).isEqualTo(TIMESTAMP); + assertThat(registration.getGasLimit()).isEqualTo(gasLimit); }) .map(ValidatorRegistration::getPublicKey) .containsExactlyInAnyOrderElementsOf( expectedRegisteredValidators.stream() - .map( - validator -> { - final BLSPublicKey publicKey = validator.getPublicKey(); - return expectedPublicKeyOverrides.getOrDefault(publicKey, publicKey); - }) + .map(Validator::getPublicKey) .collect(Collectors.toList())); } + + private SignedValidatorRegistration createSignedValidatorRegistration( + final BLSPublicKey publicKey) { + return SIGNED_VALIDATOR_REGISTRATION_SCHEMA.create( + VALIDATOR_REGISTRATION_SCHEMA.create(eth1Address, gasLimit, TIMESTAMP, publicKey), + dataStructureUtil.randomSignature()); + } } From 4495f7c993e2b79121b43e52b2936775585a1f43 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Thu, 7 Sep 2023 19:04:40 +0400 Subject: [PATCH 03/18] Cache validator statuses with ValidatorDataProvider and reuse it in validator registrations --- .../coordinator/ValidatorApiHandler.java | 50 +------ .../coordinator/ValidatorApiHandlerTest.java | 64 --------- .../teku/api/ValidatorDataProvider.java | 32 ++++- .../teku/api/ValidatorDataProviderTest.java | 79 +++++++++++ .../response/v1/beacon/ValidatorStatus.java | 6 - .../validator/api/ValidatorApiChannel.java | 9 ++ ...va => DefaultValidatorStatusProvider.java} | 111 ++++----------- .../client/ValidatorClientService.java | 28 +++- .../client/ValidatorRegistrator.java | 48 ++++--- .../client/ValidatorStatusLogger.java | 93 +++++++++++- .../client/ValidatorStatusProvider.java | 28 ++++ .../client/ValidatorStatusesChannel.java | 23 +++ .../client/ValidatorTimingActions.java | 8 +- .../client/ValidatorRegistratorTest.java | 132 ++++++++---------- ....java => ValidatorStatusProviderTest.java} | 19 +-- 15 files changed, 418 insertions(+), 312 deletions(-) rename validator/client/src/main/java/tech/pegasys/teku/validator/client/{DefaultValidatorStatusLogger.java => DefaultValidatorStatusProvider.java} (56%) create mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java create mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusesChannel.java rename validator/client/src/test/java/tech/pegasys/teku/validator/client/{DefaultValidatorStatusLoggerTest.java => ValidatorStatusProviderTest.java} (89%) diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandler.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandler.java index 000ca176f52..264ec8fa73e 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandler.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandler.java @@ -58,7 +58,6 @@ import tech.pegasys.teku.spec.datastructures.blocks.SignedBlockAndState; import tech.pegasys.teku.spec.datastructures.blocks.SignedBlockContainer; import tech.pegasys.teku.spec.datastructures.builder.SignedValidatorRegistration; -import tech.pegasys.teku.spec.datastructures.builder.ValidatorRegistration; import tech.pegasys.teku.spec.datastructures.genesis.GenesisData; import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.AttestationData; @@ -631,26 +630,8 @@ public SafeFuture prepareBeaconProposer( @Override public SafeFuture registerValidators( final SszList validatorRegistrations) { - final List validatorIdentifiers = - validatorRegistrations.stream() - .map(SignedValidatorRegistration::getMessage) - .map(ValidatorRegistration::getPublicKey) - .collect(toList()); - return getValidatorStatuses(validatorIdentifiers) - .thenCompose( - maybeValidatorStatuses -> { - if (maybeValidatorStatuses.isEmpty()) { - final String errorMessage = - "Couldn't retrieve validator statuses during registering. Most likely the BN is still syncing."; - return SafeFuture.failedFuture(new IllegalStateException(errorMessage)); - } - final SszList applicableValidatorRegistrations = - getApplicableValidatorRegistrations( - validatorRegistrations, maybeValidatorStatuses.get()); - - return proposersDataManager.updateValidatorRegistrations( - applicableValidatorRegistrations, combinedChainDataClient.getCurrentSlot()); - }); + return proposersDataManager.updateValidatorRegistrations( + validatorRegistrations, combinedChainDataClient.getCurrentSlot()); } @Override @@ -801,33 +782,6 @@ private Optional getSyncCommitteeDuty( state.getValidators().get(validatorIndex).getPublicKey(), validatorIndex, duties)); } - private SszList getApplicableValidatorRegistrations( - final SszList validatorRegistrations, - final Map validatorStatuses) { - final List applicableValidatorRegistrations = - validatorRegistrations.stream() - .filter( - signedValidatorRegistration -> { - final BLSPublicKey validatorIdentifier = - signedValidatorRegistration.getMessage().getPublicKey(); - final boolean unknownOrHasExited = - Optional.ofNullable(validatorStatuses.get(validatorIdentifier)) - .map(ValidatorStatus::hasExited) - .orElse(true); - if (unknownOrHasExited) { - LOG.debug( - "Validator {} is unknown or has exited. It will be skipped for registering.", - validatorIdentifier.toAbbreviatedString()); - } - return !unknownOrHasExited; - }) - .collect(toList()); - if (validatorRegistrations.size() == applicableValidatorRegistrations.size()) { - return validatorRegistrations; - } - return validatorRegistrations.getSchema().createFromElements(applicableValidatorRegistrations); - } - private static Optional combine( Optional a, Optional b, BiFunction fun) { if (a.isEmpty() || b.isEmpty()) { diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java index efdd390831a..ff576ce69b1 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java @@ -41,7 +41,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Optional; import java.util.concurrent.ExecutionException; import java.util.stream.Collectors; @@ -49,7 +48,6 @@ import org.apache.tuweni.bytes.Bytes32; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import tech.pegasys.teku.api.ChainDataProvider; import tech.pegasys.teku.api.NodeDataProvider; @@ -61,7 +59,6 @@ import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.bls.BLSSignature; import tech.pegasys.teku.infrastructure.async.SafeFuture; -import tech.pegasys.teku.infrastructure.async.SafeFutureAssert; import tech.pegasys.teku.infrastructure.ssz.SszList; import tech.pegasys.teku.infrastructure.ssz.SszMutableList; import tech.pegasys.teku.infrastructure.unsigned.UInt64; @@ -1058,67 +1055,6 @@ void registerValidators_shouldUpdateRegistrations() { verify(proposersDataManager).updateValidatorRegistrations(validatorRegistrations, ONE); } - @Test - void registerValidators_shouldIgnoreExitedAndUnknownValidators() { - final int numOfValidatorRegistrationsAttempted = ValidatorStatus.values().length + 2; - - final SszList validatorRegistrations = - dataStructureUtil.randomSignedValidatorRegistrations(numOfValidatorRegistrationsAttempted); - - final Map knownValidators = - IntStream.range(0, ValidatorStatus.values().length) - .mapToObj( - statusIdx -> - Map.entry( - validatorRegistrations.get(statusIdx).getMessage().getPublicKey(), - ValidatorStatus.values()[statusIdx])) - .collect(Collectors.toUnmodifiableMap(Entry::getKey, Entry::getValue)); - - final List exitedOrUnknownKeys = - IntStream.range( - ValidatorStatus.exited_unslashed.ordinal(), numOfValidatorRegistrationsAttempted) - .mapToObj( - statusOrdinal -> - validatorRegistrations.get(statusOrdinal).getMessage().getPublicKey()) - .collect(Collectors.toUnmodifiableList()); - - setupValidatorsState(validatorRegistrations, ValidatorStatus.values().length, knownValidators); - - when(chainDataClient.getCurrentSlot()).thenReturn(ONE); - - final SafeFuture result = validatorApiHandler.registerValidators(validatorRegistrations); - - assertThat(result).isCompleted(); - - @SuppressWarnings("unchecked") - final ArgumentCaptor> argumentCaptor = - ArgumentCaptor.forClass(SszList.class); - - verify(proposersDataManager).updateValidatorRegistrations(argumentCaptor.capture(), eq(ONE)); - - final SszList capturedRegistrations = argumentCaptor.getValue(); - - assertThat(capturedRegistrations) - .hasSize(5) - .map(signedRegistration -> signedRegistration.getMessage().getPublicKey()) - .doesNotContainAnyElementsOf(exitedOrUnknownKeys); - } - - @Test - void registerValidators_shouldReportErrorIfCannotRetrieveValidatorStatuses() { - final SszList validatorRegistrations = - dataStructureUtil.randomSignedValidatorRegistrations(4); - - when(chainDataProvider.getStateValidators(eq("head"), any(), any())) - .thenReturn(SafeFuture.completedFuture(Optional.empty())); - - final SafeFuture result = validatorApiHandler.registerValidators(validatorRegistrations); - - SafeFutureAssert.assertThatSafeFuture(result) - .isCompletedExceptionallyWithMessage( - "Couldn't retrieve validator statuses during registering. Most likely the BN is still syncing."); - } - @Test public void checkValidatorsDoppelganger_ShouldReturnEmptyResult() throws ExecutionException, InterruptedException { diff --git a/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java b/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java index 0a864e4e3d8..448f57e670c 100644 --- a/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java +++ b/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java @@ -23,6 +23,7 @@ import java.util.Collection; import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; import org.apache.tuweni.bytes.Bytes32; import tech.pegasys.teku.api.exceptions.BadRequestException; import tech.pegasys.teku.api.schema.SignedBeaconBlock; @@ -300,7 +301,36 @@ public SafeFuture prepareBeaconProposer( public SafeFuture registerValidators( SszList validatorRegistrations) { - return validatorApiChannel.registerValidators(validatorRegistrations); + return validatorApiChannel + .getValidatorStatuses( + validatorRegistrations.stream() + .map(registration -> registration.getMessage().getPublicKey()) + .collect(Collectors.toList())) + .thenCompose( + maybeValidatorStatuses -> { + if (maybeValidatorStatuses.isEmpty()) { + final String errorMessage = + "Couldn't retrieve validator statuses during registering. Most likely the BN is still syncing."; + return SafeFuture.failedFuture(new IllegalStateException(errorMessage)); + } + + final List activeAndPendingValidatorRegistrations = + validatorRegistrations.stream() + .filter( + registration -> + Optional.ofNullable( + maybeValidatorStatuses + .get() + .get(registration.getMessage().getPublicKey())) + .map(status -> !status.hasExited()) + .orElse(false)) + .toList(); + + return validatorApiChannel.registerValidators( + validatorRegistrations + .getSchema() + .createFromElements(activeAndPendingValidatorRegistrations)); + }); } public boolean isPhase0Slot(final UInt64 slot) { diff --git a/data/provider/src/test/java/tech/pegasys/teku/api/ValidatorDataProviderTest.java b/data/provider/src/test/java/tech/pegasys/teku/api/ValidatorDataProviderTest.java index 262a92c914b..0a6b91c6a08 100644 --- a/data/provider/src/test/java/tech/pegasys/teku/api/ValidatorDataProviderTest.java +++ b/data/provider/src/test/java/tech/pegasys/teku/api/ValidatorDataProviderTest.java @@ -20,6 +20,7 @@ import static org.assertj.core.api.Assertions.fail; import static org.assertj.core.api.Assumptions.assumeThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyCollection; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -33,27 +34,35 @@ import com.fasterxml.jackson.core.JsonProcessingException; import it.unimi.dsi.fastutil.ints.IntList; +import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.IntStream; import java.util.stream.Stream; import org.apache.tuweni.bytes.Bytes32; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.TestTemplate; import org.mockito.ArgumentCaptor; import tech.pegasys.teku.api.exceptions.BadRequestException; +import tech.pegasys.teku.api.response.v1.beacon.ValidatorStatus; import tech.pegasys.teku.api.schema.ValidatorBlockResult; import tech.pegasys.teku.api.schema.altair.SignedBeaconBlockAltair; import tech.pegasys.teku.api.schema.bellatrix.SignedBeaconBlockBellatrix; import tech.pegasys.teku.api.schema.capella.SignedBeaconBlockCapella; import tech.pegasys.teku.api.schema.deneb.SignedBeaconBlockDeneb; import tech.pegasys.teku.api.schema.phase0.SignedBeaconBlockPhase0; +import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.bls.BLSSignature; import tech.pegasys.teku.bls.BLSTestUtil; import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.async.SafeFutureAssert; import tech.pegasys.teku.infrastructure.ssz.SszData; +import tech.pegasys.teku.infrastructure.ssz.SszList; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.provider.JsonProvider; import tech.pegasys.teku.spec.Spec; @@ -63,6 +72,7 @@ import tech.pegasys.teku.spec.datastructures.blocks.BeaconBlock; import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; import tech.pegasys.teku.spec.datastructures.blocks.versions.deneb.BlockContents; +import tech.pegasys.teku.spec.datastructures.builder.SignedValidatorRegistration; import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.AttestationData; import tech.pegasys.teku.spec.util.DataStructureUtil; @@ -404,4 +414,73 @@ public void getAttesterDuties_shouldReturnDutiesForKnownValidator() { final AttesterDuties list = maybeList.orElseThrow(); assertThat(list.getDuties()).containsExactlyInAnyOrder(v1, v2); } + + @TestTemplate + void registerValidators_shouldReportErrorIfCannotRetrieveValidatorStatuses() { + final SszList validatorRegistrations = + dataStructureUtil.randomSignedValidatorRegistrations(4); + + when(validatorApiChannel.getValidatorStatuses(anyCollection())) + .thenReturn(SafeFuture.completedFuture(Optional.empty())); + + final SafeFuture result = provider.registerValidators(validatorRegistrations); + + SafeFutureAssert.assertThatSafeFuture(result) + .isCompletedExceptionallyWithMessage( + "Couldn't retrieve validator statuses during registering. Most likely the BN is still syncing."); + } + + @TestTemplate + void registerValidators_shouldIgnoreExitedAndUnknownValidators() { + final int numOfValidatorRegistrationsAttempted = ValidatorStatus.values().length + 2; + + final SszList validatorRegistrations = + dataStructureUtil.randomSignedValidatorRegistrations(numOfValidatorRegistrationsAttempted); + + final Map knownValidators = + IntStream.range(0, ValidatorStatus.values().length) + .mapToObj( + statusIdx -> + Map.entry( + validatorRegistrations.get(statusIdx).getMessage().getPublicKey(), + ValidatorStatus.values()[statusIdx])) + .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue)); + + final List exitedOrUnknownKeys = + IntStream.range( + ValidatorStatus.exited_unslashed.ordinal(), numOfValidatorRegistrationsAttempted) + .mapToObj( + statusOrdinal -> + validatorRegistrations.get(statusOrdinal).getMessage().getPublicKey()) + .toList(); + + when(validatorApiChannel.getValidatorStatuses(anyCollection())) + .thenAnswer( + args -> { + final Collection publicKeys = args.getArgument(0); + final Map validatorStatuses = + publicKeys.stream() + .filter(knownValidators::containsKey) + .collect(Collectors.toMap(Function.identity(), knownValidators::get)); + return SafeFuture.completedFuture(Optional.of(validatorStatuses)); + }); + when(validatorApiChannel.registerValidators(any())).thenReturn(SafeFuture.COMPLETE); + + final SafeFuture result = provider.registerValidators(validatorRegistrations); + + assertThat(result).isCompleted(); + + @SuppressWarnings("unchecked") + final ArgumentCaptor> argumentCaptor = + ArgumentCaptor.forClass(SszList.class); + + verify(validatorApiChannel).registerValidators(argumentCaptor.capture()); + + final SszList capturedRegistrations = argumentCaptor.getValue(); + + assertThat(capturedRegistrations) + .hasSize(5) + .map(signedRegistration -> signedRegistration.getMessage().getPublicKey()) + .doesNotContainAnyElementsOf(exitedOrUnknownKeys); + } } diff --git a/data/serializer/src/main/java/tech/pegasys/teku/api/response/v1/beacon/ValidatorStatus.java b/data/serializer/src/main/java/tech/pegasys/teku/api/response/v1/beacon/ValidatorStatus.java index ba235663e81..df0204d89ec 100644 --- a/data/serializer/src/main/java/tech/pegasys/teku/api/response/v1/beacon/ValidatorStatus.java +++ b/data/serializer/src/main/java/tech/pegasys/teku/api/response/v1/beacon/ValidatorStatus.java @@ -37,10 +37,4 @@ public enum ValidatorStatus { public boolean hasExited() { return hasExited; } - - public boolean isActive() { - return this.equals(active_ongoing) - || this.equals(active_exiting) - || this.equals(active_slashed); - } } diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorApiChannel.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorApiChannel.java index ee9b791e779..03d38f0fd48 100644 --- a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorApiChannel.java +++ b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorApiChannel.java @@ -219,6 +219,15 @@ SafeFuture sendSignedContributionAndProofs( SafeFuture prepareBeaconProposer( Collection beaconPreparableProposers); + /** + * Note that only registrations for active or pending validators must be sent to the builder + * network. Registrations for unknown or exited validators must be filtered out and not sent to + * the builder network. Expects already filtered input. + * + *

Method expects already filtered input. * *

See validator diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java index 7fdd2197860..faebac3b289 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java @@ -534,10 +534,7 @@ protected SafeFuture doStart() { eventChannels.subscribe( ValidatorTimingChannel.class, new ValidatorTimingActions( - validatorIndexProvider, - validatorTimingChannels, - spec, - metricsSystem)); + validatorIndexProvider, validatorTimingChannels, spec, metricsSystem)); validatorStatusProvider.start().ifExceptionGetsHereRaiseABug(); return beaconNodeApi.subscribeToEvents(); }); From e9bfdb24c35cfaa334d9e39f7fcbf9c7886f56e8 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Fri, 29 Sep 2023 17:14:47 +0200 Subject: [PATCH 11/18] Fixes + new test --- .../client/OwnedValidatorStatusProvider.java | 37 +++- .../client/ValidatorRegistrator.java | 5 + .../client/ValidatorRegistratorTest.java | 162 +++++++----------- .../client/ValidatorStatusProviderTest.java | 77 +++++++-- 4 files changed, 159 insertions(+), 122 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java index 48351c00865..635a8d2de8d 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java @@ -52,6 +52,7 @@ public class OwnedValidatorStatusProvider implements ValidatorStatusProvider { new AtomicReference<>(); private final AsyncRunner asyncRunner; private final AtomicBoolean startupComplete = new AtomicBoolean(false); + private final AtomicBoolean lookupInProgress = new AtomicBoolean(false); private final SettableLabelledGauge localValidatorCounts; private final AtomicReference lastRunEpoch = new AtomicReference<>(); private final AtomicReference currentEpoch = new AtomicReference<>(); @@ -129,6 +130,15 @@ private SafeFuture initValidatorStatuses() { if (validators.hasNoValidators()) { return SafeFuture.COMPLETE; } + if (currentEpoch.get() == null) { + LOG.debug("Delaying Validator status checking, currentEpoch not initialized yet"); + return retryInitialValidatorStatusCheck(); + } + + if (!lookupInProgress.compareAndSet(false, true)) { + LOG.warn("Validator status lookup is still in progress. Will skip retrying."); + return retryInitialValidatorStatusCheck(); + } // All validators are set to `unknown` until explicitly updated otherwise localValidatorCounts.set(validators.getValidatorCount(), "unknown"); return validatorApiChannel @@ -136,13 +146,19 @@ private SafeFuture initValidatorStatuses() { .thenCompose( maybeValidatorStatuses -> { if (maybeValidatorStatuses.isEmpty()) { + lookupInProgress.set(false); return retryInitialValidatorStatusCheck(); } - onNewValidatorStatuses(maybeValidatorStatuses.get()); + onNewValidatorStatuses(maybeValidatorStatuses.get(), true); startupComplete.set(true); + lookupInProgress.set(false); return SafeFuture.COMPLETE; }) - .exceptionallyCompose((__) -> retryInitialValidatorStatusCheck()); + .exceptionallyCompose( + (__) -> { + lookupInProgress.set(false); + return retryInitialValidatorStatusCheck(); + }); } private SafeFuture retryInitialValidatorStatusCheck() { @@ -154,6 +170,10 @@ public void updateValidatorStatuses() { if (!startupComplete.get() || validators.hasNoValidators()) { return; } + if (!lookupInProgress.compareAndSet(false, true)) { + LOG.warn("Validator status lookup is still in progress. Skipping update."); + return; + } if (needToUpdateAllStatuses()) { validatorApiChannel @@ -164,8 +184,9 @@ public void updateValidatorStatuses() { STATUS_LOG.unableToRetrieveValidatorStatusesFromBeaconNode(); return; } - onNewValidatorStatuses(maybeNewValidatorStatuses.get()); + onNewValidatorStatuses(maybeNewValidatorStatuses.get(), true); }) + .alwaysRun(() -> lookupInProgress.set(false)) .finish(error -> LOG.error("Failed to update validator statuses", error)); } else { final Set keysToUpdate = @@ -187,17 +208,21 @@ public void updateValidatorStatuses() { final Map oldStatuses = Optional.ofNullable(latestValidatorStatuses.get()).orElse(Map.of()); newStatuses.putAll(oldStatuses); - onNewValidatorStatuses(newStatuses); + onNewValidatorStatuses(newStatuses, false); }) + .alwaysRun(() -> lookupInProgress.set(false)) .finish(error -> LOG.error("Failed to update validator statuses", error)); } } private void onNewValidatorStatuses( - final Map newValidatorStatuses) { + final Map newValidatorStatuses, + final boolean updateLastRunEpoch) { latestValidatorStatuses.getAndSet(newValidatorStatuses); validatorStatusSubscribers.forEach(s -> s.onValidatorStatuses(newValidatorStatuses)); - lastRunEpoch.set(currentEpoch.get()); + if (updateLastRunEpoch) { + lastRunEpoch.set(currentEpoch.get()); + } updateValidatorCountMetrics(newValidatorStatuses); } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java index 7779df9a45d..7262338cac8 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java @@ -135,6 +135,11 @@ public int getNumberOfCachedRegistrations() { } private boolean isReadyToRegister() { + // Paranoid check + if (currentEpoch.get() == null) { + LOG.error("Current epoch is not yet set, skipping validator registrations"); + return false; + } if (validatorRegistrationPropertiesProvider.isReadyToProvideProperties()) { return true; } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java index d9212c9ebf2..7a748ff458e 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java @@ -65,8 +65,6 @@ class ValidatorRegistratorTest { mock(ValidatorRegistrationSigningService.class); private final ValidatorApiChannel validatorApiChannel = mock(ValidatorApiChannel.class); private final Signer signer = mock(Signer.class); - private final ValidatorStatusProvider validatorStatusProvider = - mock(ValidatorStatusProvider.class); private SpecContext specContext; private DataStructureUtil dataStructureUtil; @@ -76,13 +74,14 @@ class ValidatorRegistratorTest { private Validator validator2; private Validator validator3; + private Map validatorStatuses; + private Eth1Address eth1Address; private UInt64 gasLimit; private ValidatorRegistrator validatorRegistrator; @BeforeEach - @SuppressWarnings("unchecked") void setUp(final SpecContext specContext) { this.specContext = specContext; slotsPerEpoch = specContext.getSpec().getGenesisSpecConfig().getSlotsPerEpoch(); @@ -90,6 +89,15 @@ void setUp(final SpecContext specContext) { validator1 = new Validator(dataStructureUtil.randomPublicKey(), signer, Optional::empty); validator2 = new Validator(dataStructureUtil.randomPublicKey(), signer, Optional::empty); validator3 = new Validator(dataStructureUtil.randomPublicKey(), signer, Optional::empty); + // all validators are active by default + validatorStatuses = + Map.of( + validator1.getPublicKey(), + ValidatorStatus.active_ongoing, + validator2.getPublicKey(), + ValidatorStatus.active_ongoing, + validator3.getPublicKey(), + ValidatorStatus.active_ongoing); eth1Address = dataStructureUtil.randomEth1Address(); gasLimit = dataStructureUtil.randomUInt64(); @@ -98,7 +106,6 @@ void setUp(final SpecContext specContext) { new ValidatorRegistrator( specContext.getSpec(), ownedValidators, - validatorStatusProvider, proposerConfigPropertiesProvider, validatorRegistrationSigningService, validatorApiChannel, @@ -121,18 +128,6 @@ void setUp(final SpecContext specContext) { .when(signer) .signValidatorRegistration(any(ValidatorRegistration.class)); - // all validators are active - when(validatorStatusProvider.getStatuses()) - .thenReturn( - Optional.of( - Map.of( - validator1.getPublicKey(), - ValidatorStatus.active_ongoing, - validator2.getPublicKey(), - ValidatorStatus.active_ongoing, - validator3.getPublicKey(), - ValidatorStatus.active_ongoing))); - // registration is successful by default when(validatorApiChannel.registerValidators(any())).thenReturn(SafeFuture.COMPLETE); } @@ -141,26 +136,24 @@ void setUp(final SpecContext specContext) { void doesNotRegisterValidators_ifNotReady() { when(proposerConfigPropertiesProvider.isReadyToProvideProperties()).thenReturn(false); - runRegistrationFlowForEpoch(0); + runRegistrationFlowWithSubscription(0); verifyNoInteractions( ownedValidators, validatorApiChannel, validatorRegistrationSigningService, signer); } @TestTemplate - void doesNotRegisterValidators_ifNotThreeSlotsInTheEpoch() { + void doesNotRegisterValidators_ifTheSameStatusesArrivesInTheSameEpoch() { setOwnedValidators(validator1, validator2, validator3); // initially validators will be registered anyway since it's the first call - runRegistrationFlowForSlot(ZERO); + runRegistrationFlowForSlotWithSubscription(ZERO); - // 2 times: isReady + registration - verify(validatorStatusProvider, times(2)).getStatuses(); verify(validatorApiChannel).registerValidators(any()); - // after the initial call, registration should not occur if not three slots in the epoch - runRegistrationFlowForSlot( - UInt64.valueOf(slotsPerEpoch).plus(UInt64.valueOf(3))); // fourth slot in the epoch + // after the initial call, registration should not occur + // as it's the same epoch and validator set was not changed + runRegistrationFlowForSlotWithSubscription(UInt64.valueOf(3)); verifyNoMoreInteractions(validatorApiChannel); } @@ -169,14 +162,14 @@ void doesNotRegisterValidators_ifNotThreeSlotsInTheEpoch() { void cleanupsCache_ifValidatorIsNoLongerOwned() { setOwnedValidators(validator1, validator2, validator3); - runRegistrationFlowForEpoch(0); + runRegistrationFlowWithSubscription(0); assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(3); // validator1 not active anymore setOwnedValidators(validator2, validator3); - runRegistrationFlowForEpoch(1); + runRegistrationFlowWithSubscription(1); assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(2); } @@ -190,17 +183,6 @@ void doesNotRegisterValidatorsOnPossibleMissedEvents_ifNotReady() { ownedValidators, validatorRegistrationSigningService, validatorApiChannel, signer); } - @TestTemplate - void registerValidatorsOnPossibleMissedEvents() { - setOwnedValidators(validator1, validator2, validator3); - - validatorRegistrator.onPossibleMissedEvents(); - - final List registrationCall = captureRegistrationCall(); - - verifyRegistrations(registrationCall, List.of(validator1, validator2, validator3)); - } - @TestTemplate void doesNotRegisterNewlyAddedValidators_ifNotReady() { when(proposerConfigPropertiesProvider.isReadyToProvideProperties()).thenReturn(false); @@ -223,12 +205,12 @@ void doesNotRegisterNewlyAddedValidators_ifFirstCallHasNotBeenDone() { void registersNewlyAddedValidators() { setOwnedValidators(validator1); - runRegistrationFlowForEpoch(0); + runRegistrationFlowWithSubscription(0); // new validators are added setOwnedValidators(validator1, validator2, validator3); - validatorRegistrator.onValidatorsAdded(); + runRegistrationFlowWithSubscription(0); final List> registrationCalls = captureRegistrationCalls(2); @@ -257,7 +239,7 @@ void registerValidatorsEvenIfOneRegistrationSigningFails() { SafeFuture.completedFuture( createSignedValidatorRegistration(validator.getPublicKey()))); }); - runRegistrationFlowForEpoch(0); + runRegistrationFlowWithSubscription(0); reset(validatorRegistrationSigningService); when(validatorRegistrationSigningService.createSignedValidatorRegistration(any(), any(), any())) @@ -268,7 +250,7 @@ void registerValidatorsEvenIfOneRegistrationSigningFails() { SafeFuture.completedFuture( createSignedValidatorRegistration(validator.getPublicKey()))); }); - runRegistrationFlowForEpoch(1); + runRegistrationFlowWithSubscription(1); final List> registrationCalls = captureRegistrationCalls(2); @@ -277,39 +259,34 @@ void registerValidatorsEvenIfOneRegistrationSigningFails() { } @TestTemplate - @SuppressWarnings("unchecked") void doesNotRegister_ifValidatorIsExited() { setOwnedValidators(validator1, validator2, validator3); // only validator2 is active - when(validatorStatusProvider.getStatuses()) - .thenReturn( - Optional.of( - Map.of( - validator1.getPublicKey(), - ValidatorStatus.pending_initialized, - validator2.getPublicKey(), - ValidatorStatus.active_ongoing, - validator3.getPublicKey(), - ValidatorStatus.exited_unslashed))); - - runRegistrationFlowForEpoch(0); + validatorStatuses = + Map.of( + validator1.getPublicKey(), + ValidatorStatus.pending_initialized, + validator2.getPublicKey(), + ValidatorStatus.active_ongoing, + validator3.getPublicKey(), + ValidatorStatus.exited_unslashed); + + runRegistrationFlowWithSubscription(0); assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(2); // validator1, validator2 are active - when(validatorStatusProvider.getStatuses()) - .thenReturn( - Optional.of( - Map.of( - validator1.getPublicKey(), - ValidatorStatus.active_ongoing, - validator2.getPublicKey(), - ValidatorStatus.active_exiting, - validator3.getPublicKey(), - ValidatorStatus.exited_unslashed))); - - runRegistrationFlowForEpoch(1); + validatorStatuses = + Map.of( + validator1.getPublicKey(), + ValidatorStatus.active_ongoing, + validator2.getPublicKey(), + ValidatorStatus.active_exiting, + validator3.getPublicKey(), + ValidatorStatus.exited_unslashed); + + runRegistrationFlowWithSubscription(1); assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(2); @@ -323,10 +300,9 @@ void doesNotRegister_ifValidatorIsExited() { void doesNotRegister_ifValidatorStatusUnknown() { setOwnedValidators(validator1, validator2, validator3); // only validator2 information is available - when(validatorStatusProvider.getStatuses()) - .thenReturn(Optional.of(Map.of(validator2.getPublicKey(), ValidatorStatus.active_ongoing))); + validatorStatuses = Map.of(validator2.getPublicKey(), ValidatorStatus.active_ongoing); - runRegistrationFlowForEpoch(0); + runRegistrationFlowWithSubscription(0); assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(1); final List> registrationCalls = captureRegistrationCalls(1); @@ -337,7 +313,7 @@ void doesNotRegister_ifValidatorStatusUnknown() { void noRegistrationsAreSentIfEmpty() { setOwnedValidators(); - runRegistrationFlowForSlot(ZERO); + runRegistrationFlowForSlotWithSubscription(ZERO); verifyNoInteractions(validatorApiChannel, signer, validatorRegistrationSigningService); } @@ -348,18 +324,15 @@ void checksStatusesAndSendsRegistrationsInBatches() { new ValidatorRegistrator( specContext.getSpec(), ownedValidators, - validatorStatusProvider, proposerConfigPropertiesProvider, validatorRegistrationSigningService, validatorApiChannel, 2); setOwnedValidators(validator1, validator2, validator3); - runRegistrationFlowForEpoch(0); + runRegistrationFlowWithSubscription(0); assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(3); - // 2 times: isReady + registration - verify(validatorStatusProvider, times(2)).getStatuses(); final List> registrationCalls = captureRegistrationCalls(2); @@ -376,19 +349,16 @@ void stopsToSendBatchesOnFirstFailure() { new ValidatorRegistrator( specContext.getSpec(), ownedValidators, - validatorStatusProvider, proposerConfigPropertiesProvider, validatorRegistrationSigningService, validatorApiChannel, 2); setOwnedValidators(validator1, validator2, validator3); - runRegistrationFlowForEpoch(0); + runRegistrationFlowWithSubscription(0); // caching is after signing so still performed for first batch only assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(2); - // 2 times: isReady + registration - verify(validatorStatusProvider, times(2)).getStatuses(); verify(validatorApiChannel).registerValidators(any()); final List> registrationCalls = captureRegistrationCalls(1); @@ -402,23 +372,19 @@ void checksValidatorStatusWithPublicKeyOverride() { when(proposerConfigPropertiesProvider.getBuilderRegistrationPublicKeyOverride( validator2.getPublicKey())) .thenReturn(Optional.of(validator2KeyOverride)); - when(validatorStatusProvider.getStatuses()) - .thenReturn( - Optional.of( - Map.of( - validator1.getPublicKey(), - ValidatorStatus.active_ongoing, - validator2KeyOverride, - ValidatorStatus.active_ongoing, - validator3.getPublicKey(), - ValidatorStatus.active_ongoing))); + validatorStatuses = + Map.of( + validator1.getPublicKey(), + ValidatorStatus.active_ongoing, + validator2KeyOverride, + ValidatorStatus.active_ongoing, + validator3.getPublicKey(), + ValidatorStatus.active_ongoing); setOwnedValidators(validator1, validator2, validator3); - runRegistrationFlowForEpoch(0); + runRegistrationFlowWithSubscription(0); assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(3); - // 2 times: isReady + registration - verify(validatorStatusProvider, times(2)).getStatuses(); verify(validatorApiChannel).registerValidators(any()); final List> registrationCalls = captureRegistrationCalls(1); @@ -431,18 +397,14 @@ private void setOwnedValidators(final Validator... validators) { when(ownedValidators.getActiveValidators()).thenReturn(validatorsAsList); } - private void runRegistrationFlowForSlot(final UInt64 slot) { - validatorRegistrator.onSlot(slot); - } - - private void runRegistrationFlowForEpoch(final int epoch) { - // third slot in the epoch - final UInt64 slot = UInt64.valueOf(epoch).times(slotsPerEpoch).plus(2); + private void runRegistrationFlowForSlotWithSubscription(final UInt64 slot) { validatorRegistrator.onSlot(slot); + validatorRegistrator.onNewValidatorStatuses(validatorStatuses); } - private List captureRegistrationCall() { - return captureRegistrationCalls(1).get(0); + private void runRegistrationFlowWithSubscription(final int epoch) { + validatorRegistrator.onSlot(UInt64.valueOf(epoch).times(slotsPerEpoch)); + validatorRegistrator.onNewValidatorStatuses(validatorStatuses); } private List> captureRegistrationCalls(final int times) { diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java index 77955761ccd..600b60230a2 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java @@ -14,6 +14,7 @@ package tech.pegasys.teku.validator.client; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -26,7 +27,9 @@ import java.util.Optional; import java.util.OptionalDouble; import java.util.Set; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import tech.pegasys.teku.api.response.v1.beacon.ValidatorStatus; import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.bls.BLSTestUtil; @@ -35,6 +38,9 @@ import tech.pegasys.teku.infrastructure.metrics.StubLabelledGauge; import tech.pegasys.teku.infrastructure.metrics.StubMetricsSystem; import tech.pegasys.teku.infrastructure.metrics.TekuMetricCategory; +import tech.pegasys.teku.infrastructure.unsigned.UInt64; +import tech.pegasys.teku.spec.Spec; +import tech.pegasys.teku.spec.TestSpecFactory; import tech.pegasys.teku.validator.api.ValidatorApiChannel; import tech.pegasys.teku.validator.client.loader.OwnedValidators; @@ -46,14 +52,23 @@ public class ValidatorStatusProviderTest { private final Collection validatorKeys = Set.of(validatorKey); private final StubAsyncRunner asyncRunner = new StubAsyncRunner(); private final StubMetricsSystem metricsSystem = new StubMetricsSystem(); - - private final ValidatorStatusProvider provider = - new OwnedValidatorStatusProvider( - metricsSystem, - new OwnedValidators( - Map.of(validatorKey, new Validator(validatorKey, NO_OP_SIGNER, Optional::empty))), - validatorApiChannel, - asyncRunner); + final ValidatorStatusSubscriber validatorStatusSubscriber = mock(ValidatorStatusSubscriber.class); + private final Spec spec = TestSpecFactory.createDefault(); + private OwnedValidators ownedValidators; + + private ValidatorStatusProvider provider; + + @BeforeEach + void setup() { + ownedValidators = + new OwnedValidators( + Map.of(validatorKey, new Validator(validatorKey, NO_OP_SIGNER, Optional::empty))); + provider = + new OwnedValidatorStatusProvider( + metricsSystem, ownedValidators, validatorApiChannel, spec, asyncRunner); + provider.subscribeNewValidatorStatuses(validatorStatusSubscriber); + provider.onSlot(UInt64.ZERO); + } @Test @SuppressWarnings("unchecked") @@ -63,7 +78,7 @@ void shouldRetryGettingInitialValidatorStatuses() { .thenReturn(SafeFuture.completedFuture(Optional.empty())) .thenReturn(SafeFuture.completedFuture(Optional.of(Collections.EMPTY_MAP))); - assertThat(provider.initValidatorStatuses()).isNotCompleted(); + assertThat(provider.start()).isNotCompleted(); verify(validatorApiChannel).getValidatorStatuses(validatorKeys); asyncRunner.executeQueuedActions(); @@ -73,10 +88,8 @@ void shouldRetryGettingInitialValidatorStatuses() { asyncRunner.executeQueuedActions(); verify(validatorApiChannel, times(3)).getValidatorStatuses(validatorKeys); - - asyncRunner.executeUntilDone(); - - verify(validatorApiChannel, times(3)).getValidatorStatuses(validatorKeys); + verify(validatorStatusSubscriber).onValidatorStatuses(anyMap()); + assertThat(provider.start()).isCompleted(); } @Test @@ -86,7 +99,7 @@ void shouldSetMetricsForInitialValidatorStatuses() { SafeFuture.completedFuture( Optional.of(Map.of(validatorKey, ValidatorStatus.pending_initialized)))); - assertThat(provider.initValidatorStatuses()).isCompleted(); + assertThat(provider.start()).isCompleted(); final StubLabelledGauge gauge = metricsSystem.getLabelledGauge(TekuMetricCategory.VALIDATOR, VALIDATOR_COUNTS_METRIC); @@ -106,7 +119,7 @@ void shouldUpdateMetricsForValidatorStatuses() { SafeFuture.completedFuture( Optional.of(Map.of(validatorKey, ValidatorStatus.active_ongoing)))); - assertThat(provider.initValidatorStatuses()).isCompleted(); + assertThat(provider.start()).isCompleted(); final StubLabelledGauge gauge = metricsSystem.getLabelledGauge(TekuMetricCategory.VALIDATOR, VALIDATOR_COUNTS_METRIC); @@ -115,11 +128,43 @@ void shouldUpdateMetricsForValidatorStatuses() { assertThat(gauge.getValue(ValidatorStatus.active_ongoing.name())) .isEqualTo(OptionalDouble.of(0)); - provider.updateValidatorStatuses(); + provider.onSlot(spec.computeStartSlotAtEpoch(UInt64.ONE).plus(1)); assertThat(gauge.getValue(ValidatorStatus.pending_initialized.name())) .isEqualTo(OptionalDouble.of(0)); assertThat(gauge.getValue(ValidatorStatus.active_ongoing.name())) .isEqualTo(OptionalDouble.of(1)); } + + @Test + void shouldFireNewStatusesOnValidatorAdded() { + when(validatorApiChannel.getValidatorStatuses(validatorKeys)) + .thenReturn( + SafeFuture.completedFuture( + Optional.of(Map.of(validatorKey, ValidatorStatus.active_ongoing)))); + + assertThat(provider.start()).isCompleted(); + @SuppressWarnings("unchecked") + final ArgumentCaptor> subscriberCapture = + ArgumentCaptor.forClass(Map.class); + verify(validatorStatusSubscriber).onValidatorStatuses(subscriberCapture.capture()); + final Map result1 = subscriberCapture.getValue(); + assertThat(result1.size()).isEqualTo(1); + assertThat(result1.keySet().stream().findFirst()).contains(validatorKey); + + final BLSPublicKey validatorKey2 = BLSTestUtil.randomPublicKey(1); + final Validator validator2 = new Validator(validatorKey2, NO_OP_SIGNER, Optional::empty); + // should check only new one + when(validatorApiChannel.getValidatorStatuses(Set.of(validatorKey2))) + .thenReturn( + SafeFuture.completedFuture( + Optional.of(Map.of(validatorKey2, ValidatorStatus.active_ongoing)))); + ownedValidators.addValidator(validator2); + + provider.onValidatorsAdded(); + verify(validatorStatusSubscriber, times(2)).onValidatorStatuses(subscriberCapture.capture()); + final Map result2 = subscriberCapture.getValue(); + assertThat(result2.size()).isEqualTo(2); + assertThat(result2.keySet()).contains(validatorKey, validatorKey2); + } } From eb61ce54fb91fdea34dd83e90368ab41e1377a15 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Fri, 29 Sep 2023 21:39:45 +0200 Subject: [PATCH 12/18] Some polishing --- .../client/OwnedValidatorStatusProvider.java | 12 +----------- .../client/ValidatorStatusProviderTest.java | 1 + 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java index 635a8d2de8d..20ea7629ac4 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java @@ -135,10 +135,6 @@ private SafeFuture initValidatorStatuses() { return retryInitialValidatorStatusCheck(); } - if (!lookupInProgress.compareAndSet(false, true)) { - LOG.warn("Validator status lookup is still in progress. Will skip retrying."); - return retryInitialValidatorStatusCheck(); - } // All validators are set to `unknown` until explicitly updated otherwise localValidatorCounts.set(validators.getValidatorCount(), "unknown"); return validatorApiChannel @@ -146,19 +142,13 @@ private SafeFuture initValidatorStatuses() { .thenCompose( maybeValidatorStatuses -> { if (maybeValidatorStatuses.isEmpty()) { - lookupInProgress.set(false); return retryInitialValidatorStatusCheck(); } onNewValidatorStatuses(maybeValidatorStatuses.get(), true); startupComplete.set(true); - lookupInProgress.set(false); return SafeFuture.COMPLETE; }) - .exceptionallyCompose( - (__) -> { - lookupInProgress.set(false); - return retryInitialValidatorStatusCheck(); - }); + .exceptionallyCompose((__) -> retryInitialValidatorStatusCheck()); } private SafeFuture retryInitialValidatorStatusCheck() { diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java index 600b60230a2..a783add4505 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java @@ -152,6 +152,7 @@ void shouldFireNewStatusesOnValidatorAdded() { assertThat(result1.size()).isEqualTo(1); assertThat(result1.keySet().stream().findFirst()).contains(validatorKey); + // Adding new validator final BLSPublicKey validatorKey2 = BLSTestUtil.randomPublicKey(1); final Validator validator2 = new Validator(validatorKey2, NO_OP_SIGNER, Optional::empty); // should check only new one From 7ddd4f968c24376d834b4940639b71f601a6b0c5 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Fri, 29 Sep 2023 22:10:52 +0200 Subject: [PATCH 13/18] Fixed ValidatorStatusProvider was not subscribed to events --- .../pegasys/teku/validator/client/ValidatorClientService.java | 1 + 1 file changed, 1 insertion(+) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java index faebac3b289..323ce17fa6c 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java @@ -400,6 +400,7 @@ private void scheduleValidatorsDuties( ValidatorClientConfiguration config, ValidatorApiChannel validatorApiChannel, AsyncRunner asyncRunner) { + validatorTimingChannels.add(validatorStatusProvider); final OwnedValidators validators = validatorLoader.getOwnedValidators(); final BlockContainerSigner blockContainerSigner = new MilestoneBasedBlockContainerSigner(spec); final BlockDutyFactory blockDutyFactory = From e374caad0bca1e6dd781d2a7844061f01d414ad4 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Mon, 2 Oct 2023 23:55:26 +0400 Subject: [PATCH 14/18] Log line updated --- .../pegasys/teku/validator/client/ValidatorRegistrator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java index 7262338cac8..808ad0aa76a 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java @@ -137,7 +137,7 @@ public int getNumberOfCachedRegistrations() { private boolean isReadyToRegister() { // Paranoid check if (currentEpoch.get() == null) { - LOG.error("Current epoch is not yet set, skipping validator registrations"); + LOG.warn("Current epoch is not yet set, validator registrations delayed"); return false; } if (validatorRegistrationPropertiesProvider.isReadyToProvideProperties()) { From 3198bdf9c417afd7dec0ee92acecb897de2cabf7 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Tue, 3 Oct 2023 01:26:33 +0400 Subject: [PATCH 15/18] Refactor validator builder public key with override to use one function in all places --- .../ValidatorRegistrationSigningService.java | 6 +++--- .../client/ValidatorRegistrator.java | 21 ++++++++++++------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningService.java index ab1e4dfc910..6d62a208c83 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningService.java @@ -13,6 +13,8 @@ package tech.pegasys.teku.validator.client; +import static tech.pegasys.teku.validator.client.ValidatorRegistrator.VALIDATOR_BUILDER_PUBLICKEY; + import java.util.Optional; import java.util.function.Consumer; import org.apache.logging.log4j.LogManager; @@ -77,12 +79,10 @@ private Optional> createSignedValidatorR final Optional maybeTimestampOverride = validatorRegistrationPropertiesProvider.getBuilderRegistrationTimestampOverride(publicKey); - final Optional maybePublicKeyOverride = - validatorRegistrationPropertiesProvider.getBuilderRegistrationPublicKeyOverride(publicKey); final ValidatorRegistration validatorRegistration = createValidatorRegistration( - maybePublicKeyOverride.orElse(publicKey), + VALIDATOR_BUILDER_PUBLICKEY.apply(validator, validatorRegistrationPropertiesProvider), feeRecipient, gasLimit, maybeTimestampOverride.orElse(timeProvider.getTimeInSeconds())); diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java index 808ad0aa76a..6dd15bc1dfe 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java @@ -26,7 +26,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Function; +import java.util.function.BiFunction; import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.commons.lang3.tuple.Pair; @@ -48,6 +48,14 @@ import tech.pegasys.teku.validator.client.loader.OwnedValidators; public class ValidatorRegistrator implements ValidatorTimingChannel { + + static final BiFunction + VALIDATOR_BUILDER_PUBLICKEY = + (validator, propertiesProvider) -> + propertiesProvider + .getBuilderRegistrationPublicKeyOverride(validator.getPublicKey()) + .orElse(validator.getPublicKey()); + private static final Logger LOG = LogManager.getLogger(); private final Map cachedValidatorRegistrations = @@ -235,15 +243,14 @@ private SafeFuture processInBatches(final List validators) { private List filterActiveAndPendingValidators( final Map statuses) { - final Function getKey = - validator -> - validatorRegistrationPropertiesProvider - .getBuilderRegistrationPublicKeyOverride(validator.getPublicKey()) - .orElse(validator.getPublicKey()); + return ownedValidators.getActiveValidators().stream() .filter( validator -> - Optional.ofNullable(statuses.get(getKey.apply(validator))) + Optional.ofNullable( + statuses.get( + VALIDATOR_BUILDER_PUBLICKEY.apply( + validator, validatorRegistrationPropertiesProvider))) .map(status -> !status.hasExited()) .orElse(false)) .toList(); From e9537bb8be355312b4221de7fa07bb471afe3f2e Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Fri, 6 Oct 2023 19:18:21 +0400 Subject: [PATCH 16/18] Renaming of several methods and classes + test update --- .../client/OwnedValidatorStatusProvider.java | 10 +++--- ...> SignedValidatorRegistrationFactory.java} | 4 +-- .../client/ValidatorClientService.java | 10 +++--- .../client/ValidatorRegistrator.java | 10 +++--- .../client/ValidatorStatusLogger.java | 2 +- .../client/ValidatorStatusProvider.java | 2 +- ...gnedValidatorRegistrationFactoryTest.java} | 36 +++++++++---------- .../client/ValidatorRegistratorTest.java | 34 +++++++++--------- .../client/ValidatorStatusProviderTest.java | 6 ++-- 9 files changed, 58 insertions(+), 56 deletions(-) rename validator/client/src/main/java/tech/pegasys/teku/validator/client/{ValidatorRegistrationSigningService.java => SignedValidatorRegistrationFactory.java} (98%) rename validator/client/src/test/java/tech/pegasys/teku/validator/client/{ValidatorRegistrationSigningServiceTest.java => SignedValidatorRegistrationFactoryTest.java} (91%) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java index 20ea7629ac4..c6a1fa9a317 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java @@ -122,7 +122,7 @@ public void onAttestationCreationDue(UInt64 slot) {} public void onAttestationAggregationDue(UInt64 slot) {} @Override - public void subscribeNewValidatorStatuses(final ValidatorStatusSubscriber subscriber) { + public void subscribeValidatorStatusesUpdates(final ValidatorStatusSubscriber subscriber) { validatorStatusSubscribers.subscribe(subscriber); } @@ -144,7 +144,7 @@ private SafeFuture initValidatorStatuses() { if (maybeValidatorStatuses.isEmpty()) { return retryInitialValidatorStatusCheck(); } - onNewValidatorStatuses(maybeValidatorStatuses.get(), true); + onUpdatedValidatorStatuses(maybeValidatorStatuses.get(), true); startupComplete.set(true); return SafeFuture.COMPLETE; }) @@ -174,7 +174,7 @@ public void updateValidatorStatuses() { STATUS_LOG.unableToRetrieveValidatorStatusesFromBeaconNode(); return; } - onNewValidatorStatuses(maybeNewValidatorStatuses.get(), true); + onUpdatedValidatorStatuses(maybeNewValidatorStatuses.get(), true); }) .alwaysRun(() -> lookupInProgress.set(false)) .finish(error -> LOG.error("Failed to update validator statuses", error)); @@ -198,14 +198,14 @@ public void updateValidatorStatuses() { final Map oldStatuses = Optional.ofNullable(latestValidatorStatuses.get()).orElse(Map.of()); newStatuses.putAll(oldStatuses); - onNewValidatorStatuses(newStatuses, false); + onUpdatedValidatorStatuses(newStatuses, false); }) .alwaysRun(() -> lookupInProgress.set(false)) .finish(error -> LOG.error("Failed to update validator statuses", error)); } } - private void onNewValidatorStatuses( + private void onUpdatedValidatorStatuses( final Map newValidatorStatuses, final boolean updateLastRunEpoch) { latestValidatorStatuses.getAndSet(newValidatorStatuses); diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/SignedValidatorRegistrationFactory.java similarity index 98% rename from validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningService.java rename to validator/client/src/main/java/tech/pegasys/teku/validator/client/SignedValidatorRegistrationFactory.java index 6d62a208c83..3ca010e7ed4 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/SignedValidatorRegistrationFactory.java @@ -29,13 +29,13 @@ import tech.pegasys.teku.spec.schemas.ApiSchemas; import tech.pegasys.teku.spec.signatures.Signer; -public class ValidatorRegistrationSigningService { +public class SignedValidatorRegistrationFactory { private static final Logger LOG = LogManager.getLogger(); private final ProposerConfigPropertiesProvider validatorRegistrationPropertiesProvider; private final TimeProvider timeProvider; - public ValidatorRegistrationSigningService( + public SignedValidatorRegistrationFactory( final ProposerConfigPropertiesProvider validatorRegistrationPropertiesProvider, final TimeProvider timeProvider) { this.validatorRegistrationPropertiesProvider = validatorRegistrationPropertiesProvider; diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java index 323ce17fa6c..dfe749129ba 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java @@ -187,12 +187,12 @@ public static ValidatorClientService create( config.getSpec(), validatorLoader.getOwnedValidators(), proposerConfigManager.get(), - new ValidatorRegistrationSigningService( + new SignedValidatorRegistrationFactory( proposerConfigManager.get(), services.getTimeProvider()), validatorApiChannel, validatorConfig.getBuilderRegistrationSendingBatchSize()); - validatorStatusProvider.subscribeNewValidatorStatuses( - validatorRegistratorImpl::onNewValidatorStatuses); + validatorStatusProvider.subscribeValidatorStatusesUpdates( + validatorRegistratorImpl::onUpdatedValidatorStatuses); validatorRegistrator = Optional.of(validatorRegistratorImpl); } else { proposerConfigManager = Optional.empty(); @@ -468,8 +468,8 @@ private void scheduleValidatorsDuties( } addValidatorCountMetric(metricsSystem, validators); final ValidatorStatusLogger validatorStatusLogger = new ValidatorStatusLogger(validators); - validatorStatusProvider.subscribeNewValidatorStatuses( - validatorStatusLogger::onNewValidatorStatuses); + validatorStatusProvider.subscribeValidatorStatusesUpdates( + validatorStatusLogger::onUpdatedValidatorStatuses); } public static Path getSlashingProtectionPath(final DataDirLayout dataDirLayout) { diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java index 6dd15bc1dfe..f70326df809 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java @@ -69,7 +69,7 @@ public class ValidatorRegistrator implements ValidatorTimingChannel { private final Spec spec; private final OwnedValidators ownedValidators; private final ProposerConfigPropertiesProvider validatorRegistrationPropertiesProvider; - private final ValidatorRegistrationSigningService validatorRegistrationSigningService; + private final SignedValidatorRegistrationFactory signedValidatorRegistrationFactory; private final ValidatorApiChannel validatorApiChannel; private final int batchSize; @@ -77,13 +77,13 @@ public ValidatorRegistrator( final Spec spec, final OwnedValidators ownedValidators, final ProposerConfigPropertiesProvider validatorRegistrationPropertiesProvider, - final ValidatorRegistrationSigningService validatorRegistrationSigningService, + final SignedValidatorRegistrationFactory signedValidatorRegistrationFactory, final ValidatorApiChannel validatorApiChannel, final int batchSize) { this.spec = spec; this.ownedValidators = ownedValidators; this.validatorRegistrationPropertiesProvider = validatorRegistrationPropertiesProvider; - this.validatorRegistrationSigningService = validatorRegistrationSigningService; + this.signedValidatorRegistrationFactory = signedValidatorRegistrationFactory; this.validatorApiChannel = validatorApiChannel; this.batchSize = batchSize; } @@ -116,7 +116,7 @@ public void onAttestationCreationDue(final UInt64 slot) {} @Override public void onAttestationAggregationDue(final UInt64 slot) {} - public void onNewValidatorStatuses( + public void onUpdatedValidatorStatuses( final Map newValidatorStatuses) { if (!isReadyToRegister()) { return; @@ -264,7 +264,7 @@ private SafeFuture> createValidatorRegistratio validator -> { final Optional> maybeSignedValidatorRegistration = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator, Optional.ofNullable( cachedValidatorRegistrations.get(validator.getPublicKey())), diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java index 8cf58d48c1e..ff126525bdc 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java @@ -35,7 +35,7 @@ public ValidatorStatusLogger(final OwnedValidators validators) { this.validators = validators; } - public void onNewValidatorStatuses( + public void onUpdatedValidatorStatuses( final Map newValidatorStatuses) { final Map oldValidatorStatuses = latestValidatorStatuses.getAndSet(newValidatorStatuses); diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java index 238b1fe83bd..1bc6973af2d 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java @@ -20,5 +20,5 @@ public interface ValidatorStatusProvider extends ValidatorTimingChannel { SafeFuture start(); - void subscribeNewValidatorStatuses(ValidatorStatusSubscriber subscriber); + void subscribeValidatorStatusesUpdates(ValidatorStatusSubscriber subscriber); } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningServiceTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/SignedValidatorRegistrationFactoryTest.java similarity index 91% rename from validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningServiceTest.java rename to validator/client/src/test/java/tech/pegasys/teku/validator/client/SignedValidatorRegistrationFactoryTest.java index 25ab59e554f..45918ce2668 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningServiceTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/SignedValidatorRegistrationFactoryTest.java @@ -42,7 +42,7 @@ import tech.pegasys.teku.spec.util.DataStructureUtil; @TestSpecContext(milestone = SpecMilestone.BELLATRIX) -class ValidatorRegistrationSigningServiceTest { +class SignedValidatorRegistrationFactoryTest { private final TimeProvider stubTimeProvider = StubTimeProvider.withTimeInSeconds(12); private final ProposerConfigPropertiesProvider proposerConfigPropertiesProvider = @@ -52,7 +52,7 @@ class ValidatorRegistrationSigningServiceTest { private Validator validator; private Eth1Address feeRecipient; private UInt64 gasLimit; - private ValidatorRegistrationSigningService validatorRegistrationSigningService; + private SignedValidatorRegistrationFactory signedValidatorRegistrationFactory; private DataStructureUtil dataStructureUtil; @BeforeEach @@ -76,14 +76,14 @@ void setUp(final SpecContext specContext) { .when(signer) .signValidatorRegistration(any(ValidatorRegistration.class)); - validatorRegistrationSigningService = - new ValidatorRegistrationSigningService(proposerConfigPropertiesProvider, stubTimeProvider); + signedValidatorRegistrationFactory = + new SignedValidatorRegistrationFactory(proposerConfigPropertiesProvider, stubTimeProvider); } @TestTemplate public void whenCreateCalled_thenSigningRegistrationIsReturned() { final Optional> maybeSignedValidatorRegistration = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator, Optional.empty(), throwable -> {}); verify(signer).signValidatorRegistration(any()); @@ -100,7 +100,7 @@ public void whenOldRegistrationDoesNotNeedToBeUpdated_thenSignerIsNotCalled() { final SignedValidatorRegistration oldSignedValidatorRegistration = createSignedValidatorRegistration(validator.getPublicKey(), feeRecipient); final Optional> maybeSignedValidatorRegistration = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator, Optional.of(oldSignedValidatorRegistration), throwable -> {}); verify(signer, never()).signValidatorRegistration(any()); @@ -119,7 +119,7 @@ public void whenOldRegistrationNeedsToBeUpdated_thenSignerIsCalled() { createSignedValidatorRegistration( validator.getPublicKey(), dataStructureUtil.randomEth1Address()); final Optional> maybeSignedValidatorRegistration = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator, Optional.of(oldSignedValidatorRegistration), throwable -> {}); verify(signer).signValidatorRegistration(any()); @@ -138,7 +138,7 @@ public void whenSigningIsCompletedExceptionally_thenExceptionIsPropagated() { when(signer.signValidatorRegistration(any())) .thenReturn(SafeFuture.failedFuture(new RuntimeException("oops"))); final Optional> maybeSignedValidatorRegistration = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator, Optional.empty(), throwable -> {}); verify(signer).signValidatorRegistration(any()); @@ -157,7 +157,7 @@ void shouldRegisterWithTimestampOverride() { .thenReturn(Optional.of(timestampOverride)); final Optional> maybeSignedValidatorRegistration = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator, Optional.empty(), throwable -> {}); verify(signer).signValidatorRegistration(any()); assertThat(maybeSignedValidatorRegistration).isPresent(); @@ -178,7 +178,7 @@ void shouldRegisterWithPublicKeyOverride() { .thenReturn(Optional.of(publicKeyOverride)); final Optional> maybeSignedValidatorRegistration = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator, Optional.empty(), throwable -> {}); verify(signer).signValidatorRegistration(any()); assertThat(maybeSignedValidatorRegistration).isPresent(); @@ -201,7 +201,7 @@ void setsCorrectGasLimitForValidators() { // Verify validator final Optional> maybeSignedValidatorRegistration = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator, Optional.empty(), throwable -> {}); verify(signer).signValidatorRegistration(any()); assertThat(maybeSignedValidatorRegistration).isPresent(); @@ -215,7 +215,7 @@ void setsCorrectGasLimitForValidators() { // Verify validator2 final Optional> maybeSignedValidatorRegistration2 = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator2, Optional.empty(), throwable -> {}); verify(signer, times(2)).signValidatorRegistration(any()); assertThat(maybeSignedValidatorRegistration2).isPresent(); @@ -241,7 +241,7 @@ void setsCorrectTimeStampOverrideForValidators() { // Verify validator final Optional> maybeSignedValidatorRegistration = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator, Optional.empty(), throwable -> {}); verify(signer).signValidatorRegistration(any()); assertThat(maybeSignedValidatorRegistration).isPresent(); @@ -258,7 +258,7 @@ void setsCorrectTimeStampOverrideForValidators() { // Verify validator2 final Optional> maybeSignedValidatorRegistration2 = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator2, Optional.empty(), throwable -> {}); verify(signer, times(2)).signValidatorRegistration(any()); assertThat(maybeSignedValidatorRegistration2).isPresent(); @@ -283,7 +283,7 @@ void setsCorrectFeeRecipientForValidators() { // Verify validator final Optional> maybeSignedValidatorRegistration = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator, Optional.empty(), throwable -> {}); verify(signer).signValidatorRegistration(any()); assertThat(maybeSignedValidatorRegistration).isPresent(); @@ -297,7 +297,7 @@ void setsCorrectFeeRecipientForValidators() { // Verify validator2 final Optional> maybeSignedValidatorRegistration2 = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator2, Optional.empty(), throwable -> {}); verify(signer, times(2)).signValidatorRegistration(any()); assertThat(maybeSignedValidatorRegistration2).isPresent(); @@ -317,7 +317,7 @@ void doesNotCreateWhenFeeRecipientNotSpecified() { .thenReturn(Optional.empty()); final Optional> maybeSignedValidatorRegistration = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator, Optional.empty(), throwable -> {}); verify(signer, never()).signValidatorRegistration(any()); assertThat(maybeSignedValidatorRegistration).isEmpty(); @@ -330,7 +330,7 @@ void doesNotCreateWhenValidatorRegistrationNotEnabled() { .thenReturn(false); final Optional> maybeSignedValidatorRegistration = - validatorRegistrationSigningService.createSignedValidatorRegistration( + signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator, Optional.empty(), throwable -> {}); verify(signer, never()).signValidatorRegistration(any()); assertThat(maybeSignedValidatorRegistration).isEmpty(); diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java index 7a748ff458e..d997a53bd3d 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java @@ -61,8 +61,8 @@ class ValidatorRegistratorTest { private final OwnedValidators ownedValidators = mock(OwnedValidators.class); private final ProposerConfigPropertiesProvider proposerConfigPropertiesProvider = mock(ProposerConfigPropertiesProvider.class); - private final ValidatorRegistrationSigningService validatorRegistrationSigningService = - mock(ValidatorRegistrationSigningService.class); + private final SignedValidatorRegistrationFactory signedValidatorRegistrationFactory = + mock(SignedValidatorRegistrationFactory.class); private final ValidatorApiChannel validatorApiChannel = mock(ValidatorApiChannel.class); private final Signer signer = mock(Signer.class); @@ -107,11 +107,11 @@ void setUp(final SpecContext specContext) { specContext.getSpec(), ownedValidators, proposerConfigPropertiesProvider, - validatorRegistrationSigningService, + signedValidatorRegistrationFactory, validatorApiChannel, BATCH_SIZE); - when(validatorRegistrationSigningService.createSignedValidatorRegistration(any(), any(), any())) + when(signedValidatorRegistrationFactory.createSignedValidatorRegistration(any(), any(), any())) .thenAnswer( args -> { final Validator validator = (Validator) args.getArguments()[0]; @@ -139,7 +139,7 @@ void doesNotRegisterValidators_ifNotReady() { runRegistrationFlowWithSubscription(0); verifyNoInteractions( - ownedValidators, validatorApiChannel, validatorRegistrationSigningService, signer); + ownedValidators, validatorApiChannel, signedValidatorRegistrationFactory, signer); } @TestTemplate @@ -180,7 +180,7 @@ void doesNotRegisterValidatorsOnPossibleMissedEvents_ifNotReady() { validatorRegistrator.onPossibleMissedEvents(); verifyNoInteractions( - ownedValidators, validatorRegistrationSigningService, validatorApiChannel, signer); + ownedValidators, signedValidatorRegistrationFactory, validatorApiChannel, signer); } @TestTemplate @@ -190,7 +190,7 @@ void doesNotRegisterNewlyAddedValidators_ifNotReady() { validatorRegistrator.onValidatorsAdded(); verifyNoInteractions( - ownedValidators, validatorRegistrationSigningService, validatorApiChannel, signer); + ownedValidators, signedValidatorRegistrationFactory, validatorApiChannel, signer); } @TestTemplate @@ -198,7 +198,7 @@ void doesNotRegisterNewlyAddedValidators_ifFirstCallHasNotBeenDone() { validatorRegistrator.onValidatorsAdded(); verifyNoInteractions( - ownedValidators, validatorRegistrationSigningService, validatorApiChannel, signer); + ownedValidators, signedValidatorRegistrationFactory, validatorApiChannel, signer); } @TestTemplate @@ -227,8 +227,8 @@ void registersNewlyAddedValidators() { void registerValidatorsEvenIfOneRegistrationSigningFails() { setOwnedValidators(validator1, validator2, validator3); - reset(validatorRegistrationSigningService); - when(validatorRegistrationSigningService.createSignedValidatorRegistration(any(), any(), any())) + reset(signedValidatorRegistrationFactory); + when(signedValidatorRegistrationFactory.createSignedValidatorRegistration(any(), any(), any())) .thenAnswer( args -> { final Validator validator = (Validator) args.getArguments()[0]; @@ -241,8 +241,8 @@ void registerValidatorsEvenIfOneRegistrationSigningFails() { }); runRegistrationFlowWithSubscription(0); - reset(validatorRegistrationSigningService); - when(validatorRegistrationSigningService.createSignedValidatorRegistration(any(), any(), any())) + reset(signedValidatorRegistrationFactory); + when(signedValidatorRegistrationFactory.createSignedValidatorRegistration(any(), any(), any())) .thenAnswer( args -> { final Validator validator = (Validator) args.getArguments()[0]; @@ -315,7 +315,7 @@ void noRegistrationsAreSentIfEmpty() { runRegistrationFlowForSlotWithSubscription(ZERO); - verifyNoInteractions(validatorApiChannel, signer, validatorRegistrationSigningService); + verifyNoInteractions(validatorApiChannel, signer, signedValidatorRegistrationFactory); } @TestTemplate @@ -325,7 +325,7 @@ void checksStatusesAndSendsRegistrationsInBatches() { specContext.getSpec(), ownedValidators, proposerConfigPropertiesProvider, - validatorRegistrationSigningService, + signedValidatorRegistrationFactory, validatorApiChannel, 2); setOwnedValidators(validator1, validator2, validator3); @@ -350,7 +350,7 @@ void stopsToSendBatchesOnFirstFailure() { specContext.getSpec(), ownedValidators, proposerConfigPropertiesProvider, - validatorRegistrationSigningService, + signedValidatorRegistrationFactory, validatorApiChannel, 2); setOwnedValidators(validator1, validator2, validator3); @@ -399,12 +399,12 @@ private void setOwnedValidators(final Validator... validators) { private void runRegistrationFlowForSlotWithSubscription(final UInt64 slot) { validatorRegistrator.onSlot(slot); - validatorRegistrator.onNewValidatorStatuses(validatorStatuses); + validatorRegistrator.onUpdatedValidatorStatuses(validatorStatuses); } private void runRegistrationFlowWithSubscription(final int epoch) { validatorRegistrator.onSlot(UInt64.valueOf(epoch).times(slotsPerEpoch)); - validatorRegistrator.onNewValidatorStatuses(validatorStatuses); + validatorRegistrator.onUpdatedValidatorStatuses(validatorStatuses); } private List> captureRegistrationCalls(final int times) { diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java index a783add4505..bdff0dc3a72 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java @@ -66,7 +66,7 @@ void setup() { provider = new OwnedValidatorStatusProvider( metricsSystem, ownedValidators, validatorApiChannel, spec, asyncRunner); - provider.subscribeNewValidatorStatuses(validatorStatusSubscriber); + provider.subscribeValidatorStatusesUpdates(validatorStatusSubscriber); provider.onSlot(UInt64.ZERO); } @@ -110,7 +110,7 @@ void shouldSetMetricsForInitialValidatorStatuses() { } @Test - void shouldUpdateMetricsForValidatorStatuses() { + void shouldUpdateValidatorStatusesOnFirstEpochSlot() { when(validatorApiChannel.getValidatorStatuses(validatorKeys)) .thenReturn( SafeFuture.completedFuture( @@ -120,6 +120,7 @@ void shouldUpdateMetricsForValidatorStatuses() { Optional.of(Map.of(validatorKey, ValidatorStatus.active_ongoing)))); assertThat(provider.start()).isCompleted(); + verify(validatorStatusSubscriber).onValidatorStatuses(anyMap()); final StubLabelledGauge gauge = metricsSystem.getLabelledGauge(TekuMetricCategory.VALIDATOR, VALIDATOR_COUNTS_METRIC); @@ -129,6 +130,7 @@ void shouldUpdateMetricsForValidatorStatuses() { .isEqualTo(OptionalDouble.of(0)); provider.onSlot(spec.computeStartSlotAtEpoch(UInt64.ONE).plus(1)); + verify(validatorStatusSubscriber, times(2)).onValidatorStatuses(anyMap()); assertThat(gauge.getValue(ValidatorStatus.pending_initialized.name())) .isEqualTo(OptionalDouble.of(0)); From 9bc7aef471d9825ded93551324bb138e95aeb57e Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Fri, 6 Oct 2023 19:39:32 +0400 Subject: [PATCH 17/18] Fix not doing full registration on possible missing events --- .../client/OwnedValidatorStatusProvider.java | 25 +++++++++-------- .../client/ValidatorRegistrator.java | 14 +++++++--- .../client/ValidatorStatusLogger.java | 3 ++- .../client/ValidatorStatusSubscriber.java | 3 ++- .../client/ValidatorRegistratorTest.java | 21 +++++++++++++-- .../client/ValidatorStatusProviderTest.java | 27 +++++++++++++++---- 6 files changed, 69 insertions(+), 24 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java index c6a1fa9a317..002b1cb50fe 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java @@ -91,7 +91,7 @@ public void onSlot(UInt64 slot) { currentEpoch.set(epoch); final UInt64 firstSlotOfEpoch = spec.computeStartSlotAtEpoch(epoch); if (slot.equals(firstSlotOfEpoch.plus(1))) { - updateValidatorStatuses(); + updateValidatorStatuses(false); } } @@ -104,12 +104,12 @@ public void onHeadUpdate( @Override public void onPossibleMissedEvents() { - updateValidatorStatuses(); + updateValidatorStatuses(true); } @Override public void onValidatorsAdded() { - updateValidatorStatuses(); + updateValidatorStatuses(false); } @Override @@ -144,7 +144,7 @@ private SafeFuture initValidatorStatuses() { if (maybeValidatorStatuses.isEmpty()) { return retryInitialValidatorStatusCheck(); } - onUpdatedValidatorStatuses(maybeValidatorStatuses.get(), true); + onUpdatedValidatorStatuses(maybeValidatorStatuses.get(), false, true); startupComplete.set(true); return SafeFuture.COMPLETE; }) @@ -156,7 +156,7 @@ private SafeFuture retryInitialValidatorStatusCheck() { this::initValidatorStatuses, INITIAL_STATUS_CHECK_RETRY_PERIOD); } - public void updateValidatorStatuses() { + public void updateValidatorStatuses(final boolean possibleMissingEvents) { if (!startupComplete.get() || validators.hasNoValidators()) { return; } @@ -165,7 +165,7 @@ public void updateValidatorStatuses() { return; } - if (needToUpdateAllStatuses()) { + if (needToUpdateAllStatuses(possibleMissingEvents)) { validatorApiChannel .getValidatorStatuses(validators.getPublicKeys()) .thenAccept( @@ -174,7 +174,8 @@ public void updateValidatorStatuses() { STATUS_LOG.unableToRetrieveValidatorStatusesFromBeaconNode(); return; } - onUpdatedValidatorStatuses(maybeNewValidatorStatuses.get(), true); + onUpdatedValidatorStatuses( + maybeNewValidatorStatuses.get(), possibleMissingEvents, true); }) .alwaysRun(() -> lookupInProgress.set(false)) .finish(error -> LOG.error("Failed to update validator statuses", error)); @@ -198,7 +199,7 @@ public void updateValidatorStatuses() { final Map oldStatuses = Optional.ofNullable(latestValidatorStatuses.get()).orElse(Map.of()); newStatuses.putAll(oldStatuses); - onUpdatedValidatorStatuses(newStatuses, false); + onUpdatedValidatorStatuses(newStatuses, possibleMissingEvents, false); }) .alwaysRun(() -> lookupInProgress.set(false)) .finish(error -> LOG.error("Failed to update validator statuses", error)); @@ -207,17 +208,19 @@ public void updateValidatorStatuses() { private void onUpdatedValidatorStatuses( final Map newValidatorStatuses, + final boolean possibleMissingEvents, final boolean updateLastRunEpoch) { latestValidatorStatuses.getAndSet(newValidatorStatuses); - validatorStatusSubscribers.forEach(s -> s.onValidatorStatuses(newValidatorStatuses)); + validatorStatusSubscribers.forEach( + s -> s.onValidatorStatuses(newValidatorStatuses, possibleMissingEvents)); if (updateLastRunEpoch) { lastRunEpoch.set(currentEpoch.get()); } updateValidatorCountMetrics(newValidatorStatuses); } - private boolean needToUpdateAllStatuses() { - if (lastRunEpoch.get() == null) { + private boolean needToUpdateAllStatuses(final boolean possibleMissingEvents) { + if (lastRunEpoch.get() == null || possibleMissingEvents) { return true; } return currentEpoch.get().isGreaterThan(lastRunEpoch.get()); diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java index f70326df809..aef84653686 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java @@ -101,6 +101,11 @@ public void onHeadUpdate( final Bytes32 currentDutyDependentRoot, final Bytes32 headBlockRoot) {} + /** + * When possible missing events are detected, it may mean changing of BN which requires VC to run + * registrations again. This event is handled by possibleMissingEvents flag in {@link + * #onUpdatedValidatorStatuses(Map, boolean)} + */ @Override public void onPossibleMissedEvents() {} @@ -117,13 +122,14 @@ public void onAttestationCreationDue(final UInt64 slot) {} public void onAttestationAggregationDue(final UInt64 slot) {} public void onUpdatedValidatorStatuses( - final Map newValidatorStatuses) { + final Map newValidatorStatuses, + final boolean possibleMissingEvents) { if (!isReadyToRegister()) { return; } final List activeAndPendingValidators = filterActiveAndPendingValidators(newValidatorStatuses); - if (registrationNeedsToBeRun()) { + if (registrationNeedsToBeRun(possibleMissingEvents)) { registerValidators(activeAndPendingValidators, true); } else { final List newValidators = @@ -155,9 +161,9 @@ private boolean isReadyToRegister() { return false; } - private boolean registrationNeedsToBeRun() { + private boolean registrationNeedsToBeRun(final boolean possibleMissingEvents) { final boolean isFirstCall = firstCallDone.compareAndSet(false, true); - if (isFirstCall) { + if (isFirstCall || possibleMissingEvents) { return true; } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java index ff126525bdc..e0a8a71aa79 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java @@ -36,7 +36,8 @@ public ValidatorStatusLogger(final OwnedValidators validators) { } public void onUpdatedValidatorStatuses( - final Map newValidatorStatuses) { + final Map newValidatorStatuses, + final boolean possibleMissingEvents) { final Map oldValidatorStatuses = latestValidatorStatuses.getAndSet(newValidatorStatuses); // first run diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusSubscriber.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusSubscriber.java index f588cc010a6..90df8ed5fb1 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusSubscriber.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusSubscriber.java @@ -18,5 +18,6 @@ import tech.pegasys.teku.bls.BLSPublicKey; public interface ValidatorStatusSubscriber { - void onValidatorStatuses(Map validatorStatuses); + void onValidatorStatuses( + Map validatorStatuses, boolean possibleMissingEvents); } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java index d997a53bd3d..4448a6576e4 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java @@ -158,6 +158,23 @@ void doesNotRegisterValidators_ifTheSameStatusesArrivesInTheSameEpoch() { verifyNoMoreInteractions(validatorApiChannel); } + @TestTemplate + void registersValidatorsAgain_ifPossibleMissingEventsFiredInTheSameEpoch() { + setOwnedValidators(validator1, validator2, validator3); + + // initially validators will be registered anyway since it's the first call + runRegistrationFlowForSlotWithSubscription(ZERO); + + verify(validatorApiChannel).registerValidators(any()); + + // Even within the same epoch registration should occur again, when possible missing events are + // detected, because it may mean changing of BN which requires VC to run registration again + validatorRegistrator.onSlot(UInt64.valueOf(3)); + validatorRegistrator.onUpdatedValidatorStatuses(validatorStatuses, true); + + verify(validatorApiChannel, times(2)).registerValidators(any()); + } + @TestTemplate void cleanupsCache_ifValidatorIsNoLongerOwned() { setOwnedValidators(validator1, validator2, validator3); @@ -399,12 +416,12 @@ private void setOwnedValidators(final Validator... validators) { private void runRegistrationFlowForSlotWithSubscription(final UInt64 slot) { validatorRegistrator.onSlot(slot); - validatorRegistrator.onUpdatedValidatorStatuses(validatorStatuses); + validatorRegistrator.onUpdatedValidatorStatuses(validatorStatuses, false); } private void runRegistrationFlowWithSubscription(final int epoch) { validatorRegistrator.onSlot(UInt64.valueOf(epoch).times(slotsPerEpoch)); - validatorRegistrator.onUpdatedValidatorStatuses(validatorStatuses); + validatorRegistrator.onUpdatedValidatorStatuses(validatorStatuses, false); } private List> captureRegistrationCalls(final int times) { diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java index bdff0dc3a72..977ffc586ed 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java @@ -15,6 +15,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -88,7 +89,7 @@ void shouldRetryGettingInitialValidatorStatuses() { asyncRunner.executeQueuedActions(); verify(validatorApiChannel, times(3)).getValidatorStatuses(validatorKeys); - verify(validatorStatusSubscriber).onValidatorStatuses(anyMap()); + verify(validatorStatusSubscriber).onValidatorStatuses(anyMap(), eq(false)); assertThat(provider.start()).isCompleted(); } @@ -120,7 +121,7 @@ void shouldUpdateValidatorStatusesOnFirstEpochSlot() { Optional.of(Map.of(validatorKey, ValidatorStatus.active_ongoing)))); assertThat(provider.start()).isCompleted(); - verify(validatorStatusSubscriber).onValidatorStatuses(anyMap()); + verify(validatorStatusSubscriber).onValidatorStatuses(anyMap(), eq(false)); final StubLabelledGauge gauge = metricsSystem.getLabelledGauge(TekuMetricCategory.VALIDATOR, VALIDATOR_COUNTS_METRIC); @@ -130,7 +131,7 @@ void shouldUpdateValidatorStatusesOnFirstEpochSlot() { .isEqualTo(OptionalDouble.of(0)); provider.onSlot(spec.computeStartSlotAtEpoch(UInt64.ONE).plus(1)); - verify(validatorStatusSubscriber, times(2)).onValidatorStatuses(anyMap()); + verify(validatorStatusSubscriber, times(2)).onValidatorStatuses(anyMap(), eq(false)); assertThat(gauge.getValue(ValidatorStatus.pending_initialized.name())) .isEqualTo(OptionalDouble.of(0)); @@ -149,7 +150,7 @@ void shouldFireNewStatusesOnValidatorAdded() { @SuppressWarnings("unchecked") final ArgumentCaptor> subscriberCapture = ArgumentCaptor.forClass(Map.class); - verify(validatorStatusSubscriber).onValidatorStatuses(subscriberCapture.capture()); + verify(validatorStatusSubscriber).onValidatorStatuses(subscriberCapture.capture(), eq(false)); final Map result1 = subscriberCapture.getValue(); assertThat(result1.size()).isEqualTo(1); assertThat(result1.keySet().stream().findFirst()).contains(validatorKey); @@ -165,9 +166,25 @@ void shouldFireNewStatusesOnValidatorAdded() { ownedValidators.addValidator(validator2); provider.onValidatorsAdded(); - verify(validatorStatusSubscriber, times(2)).onValidatorStatuses(subscriberCapture.capture()); + verify(validatorStatusSubscriber, times(2)) + .onValidatorStatuses(subscriberCapture.capture(), eq(false)); final Map result2 = subscriberCapture.getValue(); assertThat(result2.size()).isEqualTo(2); assertThat(result2.keySet()).contains(validatorKey, validatorKey2); } + + @Test + void shouldPropagatePossibleMissingEvents() { + when(validatorApiChannel.getValidatorStatuses(validatorKeys)) + .thenReturn( + SafeFuture.completedFuture( + Optional.of(Map.of(validatorKey, ValidatorStatus.active_ongoing)))); + + assertThat(provider.start()).isCompleted(); + verify(validatorStatusSubscriber).onValidatorStatuses(anyMap(), eq(false)); + + // Firing onPossibleMissedEvents() + provider.onPossibleMissedEvents(); + verify(validatorStatusSubscriber).onValidatorStatuses(anyMap(), eq(true)); + } } From 98d046a23f0c50bad72bfffa366151664d45a793 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Fri, 6 Oct 2023 20:07:06 +0400 Subject: [PATCH 18/18] Use cache in validator statuses when possibleMissingEvents and it's the same epoch --- .../validator/client/OwnedValidatorStatusProvider.java | 10 +++++++--- .../validator/client/ValidatorStatusProviderTest.java | 4 ++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java index 002b1cb50fe..1d3349295c9 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java @@ -165,7 +165,7 @@ public void updateValidatorStatuses(final boolean possibleMissingEvents) { return; } - if (needToUpdateAllStatuses(possibleMissingEvents)) { + if (needToUpdateAllStatuses()) { validatorApiChannel .getValidatorStatuses(validators.getPublicKeys()) .thenAccept( @@ -185,6 +185,10 @@ public void updateValidatorStatuses(final boolean possibleMissingEvents) { .filter(key -> !latestValidatorStatuses.get().containsKey(key)) .collect(Collectors.toSet()); if (keysToUpdate.isEmpty()) { + if (possibleMissingEvents) { + validatorStatusSubscribers.forEach( + s -> s.onValidatorStatuses(latestValidatorStatuses.get(), true)); + } return; } validatorApiChannel @@ -219,8 +223,8 @@ private void onUpdatedValidatorStatuses( updateValidatorCountMetrics(newValidatorStatuses); } - private boolean needToUpdateAllStatuses(final boolean possibleMissingEvents) { - if (lastRunEpoch.get() == null || possibleMissingEvents) { + private boolean needToUpdateAllStatuses() { + if (lastRunEpoch.get() == null) { return true; } return currentEpoch.get().isGreaterThan(lastRunEpoch.get()); diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java index 977ffc586ed..b51baa0ceca 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java @@ -14,11 +14,13 @@ package tech.pegasys.teku.validator.client; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyCollection; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import static tech.pegasys.teku.spec.generator.signatures.NoOpLocalSigner.NO_OP_SIGNER; @@ -181,10 +183,12 @@ void shouldPropagatePossibleMissingEvents() { Optional.of(Map.of(validatorKey, ValidatorStatus.active_ongoing)))); assertThat(provider.start()).isCompleted(); + verify(validatorApiChannel).getValidatorStatuses(anyCollection()); verify(validatorStatusSubscriber).onValidatorStatuses(anyMap(), eq(false)); // Firing onPossibleMissedEvents() provider.onPossibleMissedEvents(); + verifyNoMoreInteractions(validatorApiChannel); verify(validatorStatusSubscriber).onValidatorStatuses(anyMap(), eq(true)); } }

See validator + * registration endpoint spec + */ SafeFuture registerValidators(SszList validatorRegistrations); SafeFuture>> getValidatorsLiveness( diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusLogger.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusProvider.java similarity index 56% rename from validator/client/src/main/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusLogger.java rename to validator/client/src/main/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusProvider.java index da705f29c07..d89baf14a66 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusLogger.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusProvider.java @@ -16,11 +16,9 @@ import static tech.pegasys.teku.infrastructure.logging.StatusLogger.STATUS_LOG; import java.time.Duration; -import java.util.HashMap; import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; import java.util.stream.Collectors; @@ -37,29 +35,31 @@ import tech.pegasys.teku.validator.api.ValidatorApiChannel; import tech.pegasys.teku.validator.client.loader.OwnedValidators; -public class DefaultValidatorStatusLogger implements ValidatorStatusLogger { +public class DefaultValidatorStatusProvider implements ValidatorStatusProvider { private static final Logger LOG = LogManager.getLogger(); - private static final int VALIDATOR_KEYS_PRINT_LIMIT = 20; private static final Duration INITIAL_STATUS_CHECK_RETRY_PERIOD = Duration.ofSeconds(5); - final OwnedValidators validators; - final ValidatorApiChannel validatorApiChannel; - final AtomicReference> latestValidatorStatuses = + private final OwnedValidators validators; + private final ValidatorApiChannel validatorApiChannel; + private final ValidatorStatusesChannel validatorStatusesChannel; + private final AtomicReference> latestValidatorStatuses = new AtomicReference<>(); - final AsyncRunner asyncRunner; - final AtomicBoolean startupComplete = new AtomicBoolean(false); + private final AsyncRunner asyncRunner; + private final AtomicBoolean startupComplete = new AtomicBoolean(false); private final SettableLabelledGauge localValidatorCounts; - public DefaultValidatorStatusLogger( - MetricsSystem metricsSystem, - OwnedValidators validators, - ValidatorApiChannel validatorApiChannel, - AsyncRunner asyncRunner) { + public DefaultValidatorStatusProvider( + final MetricsSystem metricsSystem, + final OwnedValidators validators, + final ValidatorApiChannel validatorApiChannel, + final ValidatorStatusesChannel validatorStatusesChannel, + final AsyncRunner asyncRunner) { this.validators = validators; this.validatorApiChannel = validatorApiChannel; this.asyncRunner = asyncRunner; - localValidatorCounts = + this.validatorStatusesChannel = validatorStatusesChannel; + this.localValidatorCounts = SettableLabelledGauge.create( metricsSystem, TekuMetricCategory.VALIDATOR, @@ -69,7 +69,7 @@ public DefaultValidatorStatusLogger( } @Override - public SafeFuture printInitialValidatorStatuses() { + public SafeFuture initValidatorStatuses() { if (validators.hasNoValidators()) { return SafeFuture.COMPLETE; } @@ -83,15 +83,12 @@ public SafeFuture printInitialValidatorStatuses() { return retryInitialValidatorStatusCheck(); } - Map validatorStatuses = maybeValidatorStatuses.get(); + final Map validatorStatuses = + maybeValidatorStatuses.get(); latestValidatorStatuses.set(validatorStatuses); - if (validators.getValidatorCount() < VALIDATOR_KEYS_PRINT_LIMIT) { - printValidatorStatusesOneByOne(validatorStatuses); - } else { - printValidatorStatusSummary(validatorStatuses); - } updateValidatorCountMetrics(validatorStatuses); + validatorStatusesChannel.onNewValidatorStatuses(validatorStatuses); startupComplete.set(true); return SafeFuture.COMPLETE; }) @@ -100,48 +97,11 @@ public SafeFuture printInitialValidatorStatuses() { private SafeFuture retryInitialValidatorStatusCheck() { return asyncRunner.runAfterDelay( - this::printInitialValidatorStatuses, INITIAL_STATUS_CHECK_RETRY_PERIOD); - } - - private void printValidatorStatusesOneByOne( - Map validatorStatuses) { - for (BLSPublicKey publicKey : validators.getPublicKeys()) { - Optional maybeValidatorStatus = - Optional.ofNullable(validatorStatuses.get(publicKey)); - maybeValidatorStatus.ifPresentOrElse( - validatorStatus -> - STATUS_LOG.validatorStatus(publicKey.toAbbreviatedString(), validatorStatus.name()), - () -> STATUS_LOG.unableToRetrieveValidatorStatus(publicKey.toAbbreviatedString())); - } - } - - private void printValidatorStatusSummary(Map validatorStatuses) { - Map validatorStatusCount = new HashMap<>(); - final AtomicInteger unknownValidatorCountReference = new AtomicInteger(0); - for (BLSPublicKey publicKey : validators.getPublicKeys()) { - Optional maybeValidatorStatus = - Optional.ofNullable(validatorStatuses.get(publicKey)); - maybeValidatorStatus.ifPresentOrElse( - status -> { - AtomicInteger count = - validatorStatusCount.computeIfAbsent(status, __ -> new AtomicInteger(0)); - count.incrementAndGet(); - }, - unknownValidatorCountReference::incrementAndGet); - } - - for (Map.Entry statusCount : validatorStatusCount.entrySet()) { - STATUS_LOG.validatorStatusSummary(statusCount.getValue().get(), statusCount.getKey().name()); - } - - final int unknownValidatorCount = unknownValidatorCountReference.get(); - if (unknownValidatorCount > 0) { - STATUS_LOG.unableToRetrieveValidatorStatusSummary(unknownValidatorCountReference.get()); - } + this::initValidatorStatuses, INITIAL_STATUS_CHECK_RETRY_PERIOD); } @Override - public void checkValidatorStatusChanges() { + public void updateValidatorStatuses() { if (!startupComplete.get() || validators.hasNoValidators()) { return; } @@ -155,38 +115,25 @@ public void checkValidatorStatusChanges() { return; } - Map newValidatorStatuses = + final Map newValidatorStatuses = maybeNewValidatorStatuses.get(); - Map oldValidatorStatuses = + final Map oldValidatorStatuses = latestValidatorStatuses.getAndSet(newValidatorStatuses); + validatorStatusesChannel.onNewValidatorStatuses(newValidatorStatuses); if (oldValidatorStatuses == null) { return; } - - for (Map.Entry entry : - newValidatorStatuses.entrySet()) { - BLSPublicKey key = entry.getKey(); - ValidatorStatus newStatus = entry.getValue(); - ValidatorStatus oldStatus = oldValidatorStatuses.get(key); - - // report the status of a new validator - if (oldStatus == null) { - STATUS_LOG.validatorStatus(key.toAbbreviatedString(), newStatus.name()); - continue; - } - if (oldStatus.equals(newStatus)) { - continue; - } - STATUS_LOG.validatorStatusChange( - oldStatus.name(), newStatus.name(), key.toAbbreviatedString()); - } - updateValidatorCountMetrics(newValidatorStatuses); }) .finish(error -> LOG.error("Failed to update validator statuses", error)); } + @Override + public Optional> getStatuses() { + return Optional.ofNullable(latestValidatorStatuses.get()); + } + private void updateValidatorCountMetrics( final Map oldValidatorStatuses) { final Map validatorCountByStatus = diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java index d106c78298d..3e75c9ee277 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java @@ -88,6 +88,7 @@ public class ValidatorClientService extends Service { private final List validatorTimingChannels = new ArrayList<>(); private ValidatorStatusLogger validatorStatusLogger; + private ValidatorStatusProvider validatorStatusProvider; private ValidatorIndexProvider validatorIndexProvider; private Optional maybeDoppelgangerDetector = Optional.empty(); private final DoppelgangerDetectionAction doppelgangerDetectionAction; @@ -105,6 +106,7 @@ private ValidatorClientService( final ValidatorLoader validatorLoader, final BeaconNodeApi beaconNodeApi, final ForkProvider forkProvider, + final ValidatorStatusProvider validatorStatusProvider, final Optional proposerConfigManager, final Optional beaconProposerPreparer, final Optional validatorRegistrator, @@ -115,6 +117,7 @@ private ValidatorClientService( this.validatorLoader = validatorLoader; this.beaconNodeApi = beaconNodeApi; this.forkProvider = forkProvider; + this.validatorStatusProvider = validatorStatusProvider; this.proposerConfigManager = proposerConfigManager; this.beaconProposerPreparer = beaconProposerPreparer; this.validatorRegistrator = validatorRegistrator; @@ -143,6 +146,13 @@ public static ValidatorClientService create( final ValidatorLoader validatorLoader = createValidatorLoader(services, config, asyncRunner); final ValidatorRestApiConfig validatorApiConfig = config.getValidatorRestApiConfig(); + final ValidatorStatusProvider validatorStatusProvider = + new DefaultValidatorStatusProvider( + services.getMetricsSystem(), + validatorLoader.getOwnedValidators(), + validatorApiChannel, + eventChannels.getPublisher(ValidatorStatusesChannel.class), + asyncRunner); final Optional proposerConfigManager; Optional beaconProposerPreparer = Optional.empty(); Optional validatorRegistrator = Optional.empty(); @@ -179,6 +189,7 @@ public static ValidatorClientService create( new ValidatorRegistrator( config.getSpec(), validatorLoader.getOwnedValidators(), + validatorStatusProvider, proposerConfigManager.get(), new ValidatorRegistrationSigningService( proposerConfigManager.get(), services.getTimeProvider()), @@ -194,6 +205,7 @@ public static ValidatorClientService create( validatorLoader, beaconNodeApi, forkProvider, + validatorStatusProvider, proposerConfigManager, beaconProposerPreparer, validatorRegistrator, @@ -455,9 +467,15 @@ private void scheduleValidatorsDuties( validatorRegistrator.ifPresent(validatorTimingChannels::add); } addValidatorCountMetric(metricsSystem, validators); - this.validatorStatusLogger = - new DefaultValidatorStatusLogger( - metricsSystem, validators, validatorApiChannel, asyncRunner); + this.validatorStatusProvider = + new DefaultValidatorStatusProvider( + metricsSystem, + validators, + validatorApiChannel, + eventChannels.getPublisher(ValidatorStatusesChannel.class), + asyncRunner); + this.validatorStatusLogger = new ValidatorStatusLogger(validators); + eventChannels.subscribe(ValidatorStatusesChannel.class, validatorStatusLogger); } public static Path getSlashingProtectionPath(final DataDirLayout dataDirLayout) { @@ -523,12 +541,12 @@ protected SafeFuture doStart() { eventChannels.subscribe( ValidatorTimingChannel.class, new ValidatorTimingActions( - validatorStatusLogger, + validatorStatusProvider, validatorIndexProvider, validatorTimingChannels, spec, metricsSystem)); - validatorStatusLogger.printInitialValidatorStatuses().ifExceptionGetsHereRaiseABug(); + validatorStatusProvider.initValidatorStatuses().ifExceptionGetsHereRaiseABug(); return beaconNodeApi.subscribeToEvents(); }); } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java index 50324a25d3e..3ff68f8055b 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java @@ -17,7 +17,6 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; -import java.util.Collection; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -34,6 +33,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.tuweni.bytes.Bytes32; +import tech.pegasys.teku.api.response.v1.beacon.ValidatorStatus; import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.ssz.SszList; @@ -60,6 +60,7 @@ public class ValidatorRegistrator implements ValidatorTimingChannel { private final Spec spec; private final OwnedValidators ownedValidators; + private final ValidatorStatusProvider validatorStatusProvider; private final ProposerConfigPropertiesProvider validatorRegistrationPropertiesProvider; private final ValidatorRegistrationSigningService validatorRegistrationSigningService; private final ValidatorApiChannel validatorApiChannel; @@ -68,12 +69,14 @@ public class ValidatorRegistrator implements ValidatorTimingChannel { public ValidatorRegistrator( final Spec spec, final OwnedValidators ownedValidators, + final ValidatorStatusProvider validatorStatusProvider, final ProposerConfigPropertiesProvider validatorRegistrationPropertiesProvider, final ValidatorRegistrationSigningService validatorRegistrationSigningService, final ValidatorApiChannel validatorApiChannel, final int batchSize) { this.spec = spec; this.ownedValidators = ownedValidators; + this.validatorStatusProvider = validatorStatusProvider; this.validatorRegistrationPropertiesProvider = validatorRegistrationPropertiesProvider; this.validatorRegistrationSigningService = validatorRegistrationSigningService; this.validatorApiChannel = validatorApiChannel; @@ -137,7 +140,8 @@ public int getNumberOfCachedRegistrations() { } private boolean isReadyToRegister() { - if (validatorRegistrationPropertiesProvider.isReadyToProvideProperties()) { + if (validatorRegistrationPropertiesProvider.isReadyToProvideProperties() + && validatorStatusProvider.getStatuses().isPresent()) { return true; } LOG.debug("Not ready to register validator(s)."); @@ -186,7 +190,14 @@ private SafeFuture registerValidators(final List validators) { } private SafeFuture processInBatches(final List validators) { - final List> batchedValidators = Lists.partition(validators, batchSize); + final List activeAndPendingValidators = filterActiveAndPendingValidators(validators); + if (activeAndPendingValidators.isEmpty()) { + LOG.info( + "All owned validators are either exited or with unknown status. Skipping registration."); + return SafeFuture.COMPLETE; + } + final List> batchedValidators = + Lists.partition(activeAndPendingValidators, batchSize); LOG.debug( "Going to prepare and send {} validator registration(s) to the Beacon Node in {} batch(es)", @@ -209,8 +220,7 @@ private SafeFuture processInBatches(final List validators) { "Starting to process validators registration batch {}/{}", currentBatch, batchedValidators.size()); - return filterActiveValidators(batch) - .thenCompose(this::createValidatorRegistrations) + return createValidatorRegistrations(batch) .thenCompose(this::sendValidatorRegistrations) .thenApply( size -> { @@ -230,24 +240,26 @@ private SafeFuture processInBatches(final List validators) { successfullySentRegistrations.get(), validators.size())); } - private SafeFuture> filterActiveValidators(final List validators) { + private List filterActiveAndPendingValidators(final List validators) { final Function getKey = validator -> validatorRegistrationPropertiesProvider .getBuilderRegistrationPublicKeyOverride(validator.getPublicKey()) .orElse(validator.getPublicKey()); - final Map validatorMap = - validators.stream().collect(Collectors.toMap(getKey, Function.identity())); - return validatorApiChannel - .getValidatorStatuses(validators.stream().map(getKey).toList()) - .thenApply( - maybeValidatorStatuses -> - maybeValidatorStatuses.map(Map::entrySet).stream() - .flatMap(Collection::stream) - .filter(statusEntry -> statusEntry.getValue().isActive()) - .map(statusEntry -> Optional.ofNullable(validatorMap.get(statusEntry.getKey()))) - .flatMap(Optional::stream) - .toList()); + final Map statuses = + validatorStatusProvider + .getStatuses() + .orElseThrow( + () -> + new IllegalStateException( + "Expecting loaded validator statuses, but data was not provided")); + return validators.stream() + .filter( + validator -> + Optional.ofNullable(statuses.get(getKey.apply(validator))) + .map(status -> !status.hasExited()) + .orElse(false)) + .toList(); } private SafeFuture> createValidatorRegistrations( diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java index 24ad666187c..19675f9ada8 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java @@ -13,11 +13,96 @@ package tech.pegasys.teku.validator.client; -import tech.pegasys.teku.infrastructure.async.SafeFuture; +import static tech.pegasys.teku.infrastructure.logging.StatusLogger.STATUS_LOG; -public interface ValidatorStatusLogger { +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import tech.pegasys.teku.api.response.v1.beacon.ValidatorStatus; +import tech.pegasys.teku.bls.BLSPublicKey; +import tech.pegasys.teku.validator.client.loader.OwnedValidators; - SafeFuture printInitialValidatorStatuses(); +public class ValidatorStatusLogger implements ValidatorStatusesChannel { + private static final int VALIDATOR_KEYS_PRINT_LIMIT = 20; - void checkValidatorStatusChanges(); + final OwnedValidators validators; + final AtomicReference> latestValidatorStatuses = + new AtomicReference<>(); + + public ValidatorStatusLogger(OwnedValidators validators) { + this.validators = validators; + } + + @Override + public void onNewValidatorStatuses( + final Map newValidatorStatuses) { + Map oldValidatorStatuses = + latestValidatorStatuses.getAndSet(newValidatorStatuses); + // first run + if (oldValidatorStatuses == null) { + if (validators.getValidatorCount() < VALIDATOR_KEYS_PRINT_LIMIT) { + printValidatorStatusesOneByOne(newValidatorStatuses); + } else { + printValidatorStatusSummary(newValidatorStatuses); + } + return; + } + + // updates + for (Map.Entry entry : newValidatorStatuses.entrySet()) { + BLSPublicKey key = entry.getKey(); + ValidatorStatus newStatus = entry.getValue(); + ValidatorStatus oldStatus = oldValidatorStatuses.get(key); + + // report the status of a new validator + if (oldStatus == null) { + STATUS_LOG.validatorStatus(key.toAbbreviatedString(), newStatus.name()); + continue; + } + if (oldStatus.equals(newStatus)) { + continue; + } + STATUS_LOG.validatorStatusChange( + oldStatus.name(), newStatus.name(), key.toAbbreviatedString()); + } + } + + private void printValidatorStatusesOneByOne( + Map validatorStatuses) { + for (BLSPublicKey publicKey : validators.getPublicKeys()) { + Optional maybeValidatorStatus = + Optional.ofNullable(validatorStatuses.get(publicKey)); + maybeValidatorStatus.ifPresentOrElse( + validatorStatus -> + STATUS_LOG.validatorStatus(publicKey.toAbbreviatedString(), validatorStatus.name()), + () -> STATUS_LOG.unableToRetrieveValidatorStatus(publicKey.toAbbreviatedString())); + } + } + + private void printValidatorStatusSummary(Map validatorStatuses) { + Map validatorStatusCount = new HashMap<>(); + final AtomicInteger unknownValidatorCountReference = new AtomicInteger(0); + for (BLSPublicKey publicKey : validators.getPublicKeys()) { + Optional maybeValidatorStatus = + Optional.ofNullable(validatorStatuses.get(publicKey)); + maybeValidatorStatus.ifPresentOrElse( + status -> { + AtomicInteger count = + validatorStatusCount.computeIfAbsent(status, __ -> new AtomicInteger(0)); + count.incrementAndGet(); + }, + unknownValidatorCountReference::incrementAndGet); + } + + for (Map.Entry statusCount : validatorStatusCount.entrySet()) { + STATUS_LOG.validatorStatusSummary(statusCount.getValue().get(), statusCount.getKey().name()); + } + + final int unknownValidatorCount = unknownValidatorCountReference.get(); + if (unknownValidatorCount > 0) { + STATUS_LOG.unableToRetrieveValidatorStatusSummary(unknownValidatorCountReference.get()); + } + } } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java new file mode 100644 index 00000000000..74dc91ed4bb --- /dev/null +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java @@ -0,0 +1,28 @@ +/* + * Copyright Consensys Software Inc., 2023 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.validator.client; + +import java.util.Map; +import java.util.Optional; +import tech.pegasys.teku.api.response.v1.beacon.ValidatorStatus; +import tech.pegasys.teku.bls.BLSPublicKey; +import tech.pegasys.teku.infrastructure.async.SafeFuture; + +public interface ValidatorStatusProvider { + SafeFuture initValidatorStatuses(); + + void updateValidatorStatuses(); + + Optional> getStatuses(); +} diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusesChannel.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusesChannel.java new file mode 100644 index 00000000000..d94137490c8 --- /dev/null +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusesChannel.java @@ -0,0 +1,23 @@ +/* + * Copyright Consensys Software Inc., 2022 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.validator.client; + +import java.util.Map; +import tech.pegasys.teku.api.response.v1.beacon.ValidatorStatus; +import tech.pegasys.teku.bls.BLSPublicKey; +import tech.pegasys.teku.infrastructure.events.VoidReturningChannelInterface; + +public interface ValidatorStatusesChannel extends VoidReturningChannelInterface { + void onNewValidatorStatuses(Map newValidatorStatuses); +} diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorTimingActions.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorTimingActions.java index dbe3c127b8f..0a846ce99f8 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorTimingActions.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorTimingActions.java @@ -25,18 +25,18 @@ public class ValidatorTimingActions implements ValidatorTimingChannel { private final ValidatorIndexProvider validatorIndexProvider; private final Collection delegates; - private final ValidatorStatusLogger statusLogger; + private final ValidatorStatusProvider validatorStatusProvider; private final Spec spec; private final SettableGauge validatorCurrentEpoch; public ValidatorTimingActions( - final ValidatorStatusLogger statusLogger, + final ValidatorStatusProvider validatorStatusProvider, final ValidatorIndexProvider validatorIndexProvider, final Collection delegates, final Spec spec, final MetricsSystem metricsSystem) { - this.statusLogger = statusLogger; + this.validatorStatusProvider = validatorStatusProvider; this.validatorIndexProvider = validatorIndexProvider; this.delegates = delegates; this.spec = spec; @@ -57,7 +57,7 @@ public void onSlot(final UInt64 slot) { final UInt64 firstSlotOfEpoch = spec.computeStartSlotAtEpoch(epoch); if (slot.equals(firstSlotOfEpoch.plus(1))) { validatorIndexProvider.lookupValidators(); - statusLogger.checkValidatorStatusChanges(); + validatorStatusProvider.updateValidatorStatuses(); } } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java index dcb8f99b5e3..f5bb25db9df 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java @@ -15,7 +15,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; @@ -33,7 +32,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.function.Function; import java.util.stream.Collectors; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.TestTemplate; @@ -67,6 +65,8 @@ class ValidatorRegistratorTest { mock(ValidatorRegistrationSigningService.class); private final ValidatorApiChannel validatorApiChannel = mock(ValidatorApiChannel.class); private final Signer signer = mock(Signer.class); + private final ValidatorStatusProvider validatorStatusProvider = + mock(ValidatorStatusProvider.class); private SpecContext specContext; private DataStructureUtil dataStructureUtil; @@ -98,6 +98,7 @@ void setUp(SpecContext specContext) { new ValidatorRegistrator( specContext.getSpec(), ownedValidators, + validatorStatusProvider, proposerConfigPropertiesProvider, validatorRegistrationSigningService, validatorApiChannel, @@ -121,17 +122,16 @@ void setUp(SpecContext specContext) { .signValidatorRegistration(any(ValidatorRegistration.class)); // all validators are active - when(validatorApiChannel.getValidatorStatuses(anyList())) - .thenAnswer( - args -> { - final List keys = (List) args.getArguments()[0]; - Map statuses = - keys.stream() - .collect( - Collectors.toMap( - Function.identity(), __ -> ValidatorStatus.active_ongoing)); - return SafeFuture.completedFuture(Optional.of(statuses)); - }); + when(validatorStatusProvider.getStatuses()) + .thenReturn( + Optional.of( + Map.of( + validator1.getPublicKey(), + ValidatorStatus.active_ongoing, + validator2.getPublicKey(), + ValidatorStatus.active_ongoing, + validator3.getPublicKey(), + ValidatorStatus.active_ongoing))); // registration is successful by default when(validatorApiChannel.registerValidators(any())).thenReturn(SafeFuture.COMPLETE); @@ -154,7 +154,8 @@ void doesNotRegisterValidators_ifNotThreeSlotsInTheEpoch() { // initially validators will be registered anyway since it's the first call runRegistrationFlowForSlot(ZERO); - verify(validatorApiChannel).getValidatorStatuses(any()); + // 2 times: isReady + registration + verify(validatorStatusProvider, times(2)).getStatuses(); verify(validatorApiChannel).registerValidators(any()); // after the initial call, registration should not occur if not three slots in the epoch @@ -277,56 +278,36 @@ void registerValidatorsEvenIfOneRegistrationSigningFails() { @TestTemplate @SuppressWarnings("unchecked") - void doesNotRegister_ifValidatorIsNotActive() { + void doesNotRegister_ifValidatorIsExited() { setOwnedValidators(validator1, validator2, validator3); // only validator2 is active - when(validatorApiChannel.getValidatorStatuses(anyList())) - .thenAnswer( - args -> { - final List keys = (List) args.getArguments()[0]; - Map statuses = - keys.stream() - .collect( - Collectors.toMap( - Function.identity(), - publicKey -> { - if (publicKey.equals(validator1.getPublicKey())) { - return ValidatorStatus.pending_initialized; - } - if (publicKey.equals(validator3.getPublicKey())) { - return ValidatorStatus.exited_unslashed; - } - return ValidatorStatus.active_ongoing; - })); - return SafeFuture.completedFuture(Optional.of(statuses)); - }); + when(validatorStatusProvider.getStatuses()) + .thenReturn( + Optional.of( + Map.of( + validator1.getPublicKey(), + ValidatorStatus.pending_initialized, + validator2.getPublicKey(), + ValidatorStatus.active_ongoing, + validator3.getPublicKey(), + ValidatorStatus.exited_unslashed))); runRegistrationFlowForEpoch(0); - assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(1); + assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(2); // validator1, validator2 are active - when(validatorApiChannel.getValidatorStatuses(anyList())) - .thenAnswer( - args -> { - final List keys = (List) args.getArguments()[0]; - Map statuses = - keys.stream() - .collect( - Collectors.toMap( - Function.identity(), - publicKey -> { - if (publicKey.equals(validator2.getPublicKey())) { - return ValidatorStatus.active_exiting; - } - if (publicKey.equals(validator3.getPublicKey())) { - return ValidatorStatus.exited_unslashed; - } - return ValidatorStatus.active_ongoing; - })); - return SafeFuture.completedFuture(Optional.of(statuses)); - }); + when(validatorStatusProvider.getStatuses()) + .thenReturn( + Optional.of( + Map.of( + validator1.getPublicKey(), + ValidatorStatus.active_ongoing, + validator2.getPublicKey(), + ValidatorStatus.active_exiting, + validator3.getPublicKey(), + ValidatorStatus.exited_unslashed))); runRegistrationFlowForEpoch(1); @@ -334,18 +315,16 @@ void doesNotRegister_ifValidatorIsNotActive() { final List> registrationCalls = captureRegistrationCalls(2); - verifyRegistrations(registrationCalls.get(0), List.of(validator2)); + verifyRegistrations(registrationCalls.get(0), List.of(validator1, validator2)); verifyRegistrations(registrationCalls.get(1), List.of(validator1, validator2)); } @TestTemplate - void doesNotRegister_ifValidatorStatusNotFound() { + void doesNotRegister_ifValidatorStatusUnknown() { setOwnedValidators(validator1, validator2, validator3); - // validator2 information is only available - when(validatorApiChannel.getValidatorStatuses(anyList())) - .thenReturn( - SafeFuture.completedFuture( - Optional.of(Map.of(validator2.getPublicKey(), ValidatorStatus.active_ongoing)))); + // only validator2 information is available + when(validatorStatusProvider.getStatuses()) + .thenReturn(Optional.of(Map.of(validator2.getPublicKey(), ValidatorStatus.active_ongoing))); runRegistrationFlowForEpoch(0); @@ -369,6 +348,7 @@ void checksStatusesAndSendsRegistrationsInBatches() { new ValidatorRegistrator( specContext.getSpec(), ownedValidators, + validatorStatusProvider, proposerConfigPropertiesProvider, validatorRegistrationSigningService, validatorApiChannel, @@ -378,9 +358,8 @@ void checksStatusesAndSendsRegistrationsInBatches() { runRegistrationFlowForEpoch(0); assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(3); - verify(validatorApiChannel) - .getValidatorStatuses(List.of(validator1.getPublicKey(), validator2.getPublicKey())); - verify(validatorApiChannel).getValidatorStatuses(List.of(validator3.getPublicKey())); + // 2 times: isReady + registration + verify(validatorStatusProvider, times(2)).getStatuses(); final List> registrationCalls = captureRegistrationCalls(2); @@ -397,6 +376,7 @@ void stopsToSendBatchesOnFirstFailure() { new ValidatorRegistrator( specContext.getSpec(), ownedValidators, + validatorStatusProvider, proposerConfigPropertiesProvider, validatorRegistrationSigningService, validatorApiChannel, @@ -407,8 +387,8 @@ void stopsToSendBatchesOnFirstFailure() { // caching is after signing so still performed for first batch only assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(2); - // 1 time only, second batch not performed - verify(validatorApiChannel).getValidatorStatuses(anyList()); + // 2 times: isReady + registration + verify(validatorStatusProvider, times(2)).getStatuses(); verify(validatorApiChannel).registerValidators(any()); final List> registrationCalls = captureRegistrationCalls(1); @@ -422,15 +402,23 @@ void checksValidatorStatusWithPublicKeyOverride() { when(proposerConfigPropertiesProvider.getBuilderRegistrationPublicKeyOverride( validator2.getPublicKey())) .thenReturn(Optional.of(validator2KeyOverride)); + when(validatorStatusProvider.getStatuses()) + .thenReturn( + Optional.of( + Map.of( + validator1.getPublicKey(), + ValidatorStatus.active_ongoing, + validator2KeyOverride, + ValidatorStatus.active_ongoing, + validator3.getPublicKey(), + ValidatorStatus.active_ongoing))); setOwnedValidators(validator1, validator2, validator3); runRegistrationFlowForEpoch(0); assertThat(validatorRegistrator.getNumberOfCachedRegistrations()).isEqualTo(3); - // 1 time only, second batch not performed - verify(validatorApiChannel) - .getValidatorStatuses( - List.of(validator1.getPublicKey(), validator2KeyOverride, validator3.getPublicKey())); + // 2 times: isReady + registration + verify(validatorStatusProvider, times(2)).getStatuses(); verify(validatorApiChannel).registerValidators(any()); final List> registrationCalls = captureRegistrationCalls(1); diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusLoggerTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java similarity index 89% rename from validator/client/src/test/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusLoggerTest.java rename to validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java index cdd09f6e1dc..3bbc6faeba2 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusLoggerTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java @@ -38,7 +38,7 @@ import tech.pegasys.teku.validator.api.ValidatorApiChannel; import tech.pegasys.teku.validator.client.loader.OwnedValidators; -public class DefaultValidatorStatusLoggerTest { +public class ValidatorStatusProviderTest { public static final String VALIDATOR_COUNTS_METRIC = "local_validator_counts"; private final ValidatorApiChannel validatorApiChannel = mock(ValidatorApiChannel.class); @@ -46,24 +46,27 @@ public class DefaultValidatorStatusLoggerTest { private final Collection validatorKeys = Set.of(validatorKey); private final StubAsyncRunner asyncRunner = new StubAsyncRunner(); private final StubMetricsSystem metricsSystem = new StubMetricsSystem(); + private final ValidatorStatusesChannel validatorStatusesChannel = + mock(ValidatorStatusesChannel.class); - private final DefaultValidatorStatusLogger logger = - new DefaultValidatorStatusLogger( + private final ValidatorStatusProvider provider = + new DefaultValidatorStatusProvider( metricsSystem, new OwnedValidators( Map.of(validatorKey, new Validator(validatorKey, NO_OP_SIGNER, Optional::empty))), validatorApiChannel, + validatorStatusesChannel, asyncRunner); @Test @SuppressWarnings("unchecked") - void shouldRetryPrintingInitialValidatorStatuses() { + void shouldRetryGettingInitialValidatorStatuses() { when(validatorApiChannel.getValidatorStatuses(validatorKeys)) .thenReturn(SafeFuture.completedFuture(Optional.empty())) .thenReturn(SafeFuture.completedFuture(Optional.empty())) .thenReturn(SafeFuture.completedFuture(Optional.of(Collections.EMPTY_MAP))); - assertThat(logger.printInitialValidatorStatuses()).isNotCompleted(); + assertThat(provider.initValidatorStatuses()).isNotCompleted(); verify(validatorApiChannel).getValidatorStatuses(validatorKeys); asyncRunner.executeQueuedActions(); @@ -86,7 +89,7 @@ void shouldSetMetricsForInitialValidatorStatuses() { SafeFuture.completedFuture( Optional.of(Map.of(validatorKey, ValidatorStatus.pending_initialized)))); - assertThat(logger.printInitialValidatorStatuses()).isCompleted(); + assertThat(provider.initValidatorStatuses()).isCompleted(); final StubLabelledGauge gauge = metricsSystem.getLabelledGauge(TekuMetricCategory.VALIDATOR, VALIDATOR_COUNTS_METRIC); @@ -106,7 +109,7 @@ void shouldUpdateMetricsForValidatorStatuses() { SafeFuture.completedFuture( Optional.of(Map.of(validatorKey, ValidatorStatus.active_ongoing)))); - assertThat(logger.printInitialValidatorStatuses()).isCompleted(); + assertThat(provider.initValidatorStatuses()).isCompleted(); final StubLabelledGauge gauge = metricsSystem.getLabelledGauge(TekuMetricCategory.VALIDATOR, VALIDATOR_COUNTS_METRIC); @@ -115,7 +118,7 @@ void shouldUpdateMetricsForValidatorStatuses() { assertThat(gauge.getValue(ValidatorStatus.active_ongoing.name())) .isEqualTo(OptionalDouble.of(0)); - logger.checkValidatorStatusChanges(); + provider.updateValidatorStatuses(); assertThat(gauge.getValue(ValidatorStatus.pending_initialized.name())) .isEqualTo(OptionalDouble.of(0)); From 5782d24570bb14216637011c4d7d29ef44b039a7 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Thu, 7 Sep 2023 19:47:12 +0400 Subject: [PATCH 04/18] Fix integration test for PostRegisterValidator --- .../validator/PostRegisterValidatorTest.java | 37 +++++++++++++++++++ .../v1/validator/PostRegisterValidator.java | 7 +++- .../teku/api/ValidatorDataProvider.java | 2 +- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v1/validator/PostRegisterValidatorTest.java b/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v1/validator/PostRegisterValidatorTest.java index 6ae5d7fede2..280a29dc00a 100644 --- a/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v1/validator/PostRegisterValidatorTest.java +++ b/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v1/validator/PostRegisterValidatorTest.java @@ -15,6 +15,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyCollection; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_BAD_REQUEST; @@ -23,13 +24,20 @@ import static tech.pegasys.teku.spec.schemas.ApiSchemas.SIGNED_VALIDATOR_REGISTRATIONS_SCHEMA; import java.io.IOException; +import java.util.Collection; +import java.util.Map; +import java.util.Optional; +import java.util.function.Function; +import java.util.stream.Collectors; import okhttp3.Response; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import tech.pegasys.teku.api.response.v1.beacon.ValidatorStatus; import tech.pegasys.teku.beaconrestapi.AbstractDataBackedRestAPIIntegrationTest; import tech.pegasys.teku.beaconrestapi.handlers.v1.validator.PostRegisterValidator; +import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.json.JsonUtil; import tech.pegasys.teku.infrastructure.ssz.SszList; @@ -44,6 +52,17 @@ public class PostRegisterValidatorTest extends AbstractDataBackedRestAPIIntegrat void setup() { startRestAPIAtGenesis(SpecMilestone.BELLATRIX); dataStructureUtil = new DataStructureUtil(spec); + when(validatorApiChannel.getValidatorStatuses(anyCollection())) + .thenAnswer( + args -> { + final Collection publicKeys = args.getArgument(0); + final Map validatorStatuses = + publicKeys.stream() + .collect( + Collectors.toMap( + Function.identity(), __ -> ValidatorStatus.active_ongoing)); + return SafeFuture.completedFuture(Optional.of(validatorStatuses)); + }); } @Test @@ -80,6 +99,24 @@ void shouldReturnServerErrorWhenThereIsAnExceptionWhileRegistering() throws IOEx } } + @Test + void shouldReturnServerErrorWhenThereIsAnExceptionWhileGettingStatuses() throws IOException { + final SszList request = + dataStructureUtil.randomSignedValidatorRegistrations(10); + when(validatorApiChannel.getValidatorStatuses(anyCollection())) + .thenReturn(SafeFuture.failedFuture(new IllegalStateException("oopsy"))); + + try (Response response = + post( + PostRegisterValidator.ROUTE, + JsonUtil.serialize( + request, SIGNED_VALIDATOR_REGISTRATIONS_SCHEMA.getJsonTypeDefinition()))) { + + assertThat(response.code()).isEqualTo(SC_INTERNAL_SERVER_ERROR); + assertThat(response.body().string()).isEqualTo("{\"code\":500,\"message\":\"oopsy\"}"); + } + } + @ParameterizedTest @ValueSource(strings = {"[{}]", "{}", "invalid"}) void shouldReturnBadRequest(final String body) throws IOException { diff --git a/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/validator/PostRegisterValidator.java b/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/validator/PostRegisterValidator.java index a2996fa3718..70352c7aa95 100644 --- a/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/validator/PostRegisterValidator.java +++ b/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/validator/PostRegisterValidator.java @@ -69,8 +69,11 @@ public void handleRequest(final RestApiRequest request) throws JsonProcessingExc .handle( (__, error) -> { if (error != null) { - return AsyncApiResponse.respondWithError( - SC_INTERNAL_SERVER_ERROR, error.getMessage()); + final String message = + error.getCause() == null + ? error.getMessage() + : error.getCause().getMessage(); + return AsyncApiResponse.respondWithError(SC_INTERNAL_SERVER_ERROR, message); } return AsyncApiResponse.respondOk(null); })); diff --git a/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java b/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java index 448f57e670c..7317e14abe0 100644 --- a/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java +++ b/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java @@ -306,7 +306,7 @@ public SafeFuture registerValidators( validatorRegistrations.stream() .map(registration -> registration.getMessage().getPublicKey()) .collect(Collectors.toList())) - .thenCompose( + .thenComposeChecked( maybeValidatorStatuses -> { if (maybeValidatorStatuses.isEmpty()) { final String errorMessage = From 039ef31a84605fb54443751c7549a379756c3b7a Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Wed, 20 Sep 2023 19:07:30 +0400 Subject: [PATCH 05/18] Remove duplicate validatorStatusProvider instance creation --- .../validator/client/ValidatorClientService.java | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java index 3e75c9ee277..7943c0443f4 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java @@ -87,8 +87,7 @@ public class ValidatorClientService extends Service { private final Spec spec; private final List validatorTimingChannels = new ArrayList<>(); - private ValidatorStatusLogger validatorStatusLogger; - private ValidatorStatusProvider validatorStatusProvider; + private final ValidatorStatusProvider validatorStatusProvider; private ValidatorIndexProvider validatorIndexProvider; private Optional maybeDoppelgangerDetector = Optional.empty(); private final DoppelgangerDetectionAction doppelgangerDetectionAction; @@ -467,14 +466,7 @@ private void scheduleValidatorsDuties( validatorRegistrator.ifPresent(validatorTimingChannels::add); } addValidatorCountMetric(metricsSystem, validators); - this.validatorStatusProvider = - new DefaultValidatorStatusProvider( - metricsSystem, - validators, - validatorApiChannel, - eventChannels.getPublisher(ValidatorStatusesChannel.class), - asyncRunner); - this.validatorStatusLogger = new ValidatorStatusLogger(validators); + final ValidatorStatusLogger validatorStatusLogger = new ValidatorStatusLogger(validators); eventChannels.subscribe(ValidatorStatusesChannel.class, validatorStatusLogger); } From 3b5f70112d3fbf6838b1b2ee0f6fcdb89ac2cfa8 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Wed, 20 Sep 2023 22:04:27 +0400 Subject: [PATCH 06/18] A lot of finals + getting error message refactoring --- .../validator/PostRegisterValidatorTest.java | 10 +++--- .../v1/validator/PostRegisterValidator.java | 8 ++--- .../teku/api/ValidatorDataProvider.java | 6 ++-- .../client/ValidatorClientService.java | 14 ++++---- .../client/ValidatorStatusLogger.java | 32 ++++++++++--------- ...lidatorRegistrationSigningServiceTest.java | 2 +- .../client/ValidatorRegistratorTest.java | 2 +- 7 files changed, 37 insertions(+), 37 deletions(-) diff --git a/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v1/validator/PostRegisterValidatorTest.java b/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v1/validator/PostRegisterValidatorTest.java index 280a29dc00a..ded367018a3 100644 --- a/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v1/validator/PostRegisterValidatorTest.java +++ b/data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v1/validator/PostRegisterValidatorTest.java @@ -71,7 +71,7 @@ void shouldReturnOk() throws IOException { dataStructureUtil.randomSignedValidatorRegistrations(10); when(validatorApiChannel.registerValidators(request)).thenReturn(SafeFuture.COMPLETE); - try (Response response = + try (final Response response = post( PostRegisterValidator.ROUTE, JsonUtil.serialize( @@ -88,7 +88,7 @@ void shouldReturnServerErrorWhenThereIsAnExceptionWhileRegistering() throws IOEx when(validatorApiChannel.registerValidators(request)) .thenReturn(SafeFuture.failedFuture(new IllegalStateException("oopsy"))); - try (Response response = + try (final Response response = post( PostRegisterValidator.ROUTE, JsonUtil.serialize( @@ -106,7 +106,7 @@ void shouldReturnServerErrorWhenThereIsAnExceptionWhileGettingStatuses() throws when(validatorApiChannel.getValidatorStatuses(anyCollection())) .thenReturn(SafeFuture.failedFuture(new IllegalStateException("oopsy"))); - try (Response response = + try (final Response response = post( PostRegisterValidator.ROUTE, JsonUtil.serialize( @@ -121,7 +121,7 @@ void shouldReturnServerErrorWhenThereIsAnExceptionWhileGettingStatuses() throws @ValueSource(strings = {"[{}]", "{}", "invalid"}) void shouldReturnBadRequest(final String body) throws IOException { when(validatorApiChannel.registerValidators(any())).thenReturn(SafeFuture.COMPLETE); - try (Response response = post(PostRegisterValidator.ROUTE, body)) { + try (final Response response = post(PostRegisterValidator.ROUTE, body)) { assertThat(response.code()).isEqualTo(SC_BAD_REQUEST); } verifyNoInteractions(validatorApiChannel); @@ -130,7 +130,7 @@ void shouldReturnBadRequest(final String body) throws IOException { @Test void shouldHandleEmptyRequest() throws IOException { when(validatorApiChannel.registerValidators(any())).thenReturn(SafeFuture.COMPLETE); - try (Response response = post(PostRegisterValidator.ROUTE, "[]")) { + try (final Response response = post(PostRegisterValidator.ROUTE, "[]")) { assertThat(response.code()).isEqualTo(SC_OK); } } diff --git a/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/validator/PostRegisterValidator.java b/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/validator/PostRegisterValidator.java index 70352c7aa95..42940ef13c9 100644 --- a/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/validator/PostRegisterValidator.java +++ b/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/validator/PostRegisterValidator.java @@ -23,6 +23,7 @@ import java.util.Optional; import tech.pegasys.teku.api.DataProvider; import tech.pegasys.teku.api.ValidatorDataProvider; +import tech.pegasys.teku.infrastructure.exceptions.ExceptionUtil; import tech.pegasys.teku.infrastructure.restapi.endpoints.AsyncApiResponse; import tech.pegasys.teku.infrastructure.restapi.endpoints.EndpointMetadata; import tech.pegasys.teku.infrastructure.restapi.endpoints.RestApiEndpoint; @@ -69,11 +70,8 @@ public void handleRequest(final RestApiRequest request) throws JsonProcessingExc .handle( (__, error) -> { if (error != null) { - final String message = - error.getCause() == null - ? error.getMessage() - : error.getCause().getMessage(); - return AsyncApiResponse.respondWithError(SC_INTERNAL_SERVER_ERROR, message); + return AsyncApiResponse.respondWithError( + SC_INTERNAL_SERVER_ERROR, ExceptionUtil.getRootCauseMessage(error)); } return AsyncApiResponse.respondOk(null); })); diff --git a/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java b/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java index 7317e14abe0..61af75420af 100644 --- a/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java +++ b/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java @@ -256,7 +256,7 @@ public SafeFuture> createAggregate( } public SafeFuture> sendAggregateAndProofs( - List aggregateAndProofs) { + final List aggregateAndProofs) { return validatorApiChannel.sendAggregateAndProofs(aggregateAndProofs); } @@ -295,12 +295,12 @@ public SafeFuture sendContributionAndProofs( } public SafeFuture prepareBeaconProposer( - List beaconPreparableProposers) { + final List beaconPreparableProposers) { return validatorApiChannel.prepareBeaconProposer(beaconPreparableProposers); } public SafeFuture registerValidators( - SszList validatorRegistrations) { + final SszList validatorRegistrations) { return validatorApiChannel .getValidatorStatuses( validatorRegistrations.stream() diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java index 7943c0443f4..2bce5040376 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java @@ -198,7 +198,7 @@ public static ValidatorClientService create( proposerConfigManager = Optional.empty(); } - ValidatorClientService validatorClientService = + final ValidatorClientService validatorClientService = new ValidatorClientService( eventChannels, validatorLoader, @@ -273,12 +273,12 @@ public static ValidatorClientService create( } private void initializeDoppelgangerDetector( - AsyncRunner asyncRunner, - ValidatorApiChannel validatorApiChannel, - Spec spec, - TimeProvider timeProvider, - GenesisDataProvider genesisDataProvider) { - DoppelgangerDetector doppelgangerDetector = + final AsyncRunner asyncRunner, + final ValidatorApiChannel validatorApiChannel, + final Spec spec, + final TimeProvider timeProvider, + final GenesisDataProvider genesisDataProvider) { + final DoppelgangerDetector doppelgangerDetector = new DoppelgangerDetector( asyncRunner, validatorApiChannel, diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java index 19675f9ada8..420c851d107 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java @@ -31,14 +31,14 @@ public class ValidatorStatusLogger implements ValidatorStatusesChannel { final AtomicReference> latestValidatorStatuses = new AtomicReference<>(); - public ValidatorStatusLogger(OwnedValidators validators) { + public ValidatorStatusLogger(final OwnedValidators validators) { this.validators = validators; } @Override public void onNewValidatorStatuses( final Map newValidatorStatuses) { - Map oldValidatorStatuses = + final Map oldValidatorStatuses = latestValidatorStatuses.getAndSet(newValidatorStatuses); // first run if (oldValidatorStatuses == null) { @@ -51,10 +51,10 @@ public void onNewValidatorStatuses( } // updates - for (Map.Entry entry : newValidatorStatuses.entrySet()) { - BLSPublicKey key = entry.getKey(); - ValidatorStatus newStatus = entry.getValue(); - ValidatorStatus oldStatus = oldValidatorStatuses.get(key); + for (final Map.Entry entry : newValidatorStatuses.entrySet()) { + final BLSPublicKey key = entry.getKey(); + final ValidatorStatus newStatus = entry.getValue(); + final ValidatorStatus oldStatus = oldValidatorStatuses.get(key); // report the status of a new validator if (oldStatus == null) { @@ -70,9 +70,9 @@ public void onNewValidatorStatuses( } private void printValidatorStatusesOneByOne( - Map validatorStatuses) { - for (BLSPublicKey publicKey : validators.getPublicKeys()) { - Optional maybeValidatorStatus = + final Map validatorStatuses) { + for (final BLSPublicKey publicKey : validators.getPublicKeys()) { + final Optional maybeValidatorStatus = Optional.ofNullable(validatorStatuses.get(publicKey)); maybeValidatorStatus.ifPresentOrElse( validatorStatus -> @@ -81,22 +81,24 @@ private void printValidatorStatusesOneByOne( } } - private void printValidatorStatusSummary(Map validatorStatuses) { - Map validatorStatusCount = new HashMap<>(); + private void printValidatorStatusSummary( + final Map validatorStatuses) { + final Map validatorStatusCount = new HashMap<>(); final AtomicInteger unknownValidatorCountReference = new AtomicInteger(0); - for (BLSPublicKey publicKey : validators.getPublicKeys()) { - Optional maybeValidatorStatus = + for (final BLSPublicKey publicKey : validators.getPublicKeys()) { + final Optional maybeValidatorStatus = Optional.ofNullable(validatorStatuses.get(publicKey)); maybeValidatorStatus.ifPresentOrElse( status -> { - AtomicInteger count = + final AtomicInteger count = validatorStatusCount.computeIfAbsent(status, __ -> new AtomicInteger(0)); count.incrementAndGet(); }, unknownValidatorCountReference::incrementAndGet); } - for (Map.Entry statusCount : validatorStatusCount.entrySet()) { + for (final Map.Entry statusCount : + validatorStatusCount.entrySet()) { STATUS_LOG.validatorStatusSummary(statusCount.getValue().get(), statusCount.getKey().name()); } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningServiceTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningServiceTest.java index 9c7be05a9a4..25ab59e554f 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningServiceTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistrationSigningServiceTest.java @@ -56,7 +56,7 @@ class ValidatorRegistrationSigningServiceTest { private DataStructureUtil dataStructureUtil; @BeforeEach - void setUp(SpecContext specContext) { + void setUp(final SpecContext specContext) { dataStructureUtil = specContext.getDataStructureUtil(); feeRecipient = dataStructureUtil.randomEth1Address(); gasLimit = dataStructureUtil.randomUInt64(); diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java index f5bb25db9df..d9212c9ebf2 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorRegistratorTest.java @@ -83,7 +83,7 @@ class ValidatorRegistratorTest { @BeforeEach @SuppressWarnings("unchecked") - void setUp(SpecContext specContext) { + void setUp(final SpecContext specContext) { this.specContext = specContext; slotsPerEpoch = specContext.getSpec().getGenesisSpecConfig().getSlotsPerEpoch(); dataStructureUtil = specContext.getDataStructureUtil(); From c53e8a5b4b2eee0697ce09b27649286002e206a0 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Wed, 20 Sep 2023 22:21:14 +0400 Subject: [PATCH 07/18] Use Subscribers model rather than Channel for new validator statuses --- validator/client/build.gradle | 1 + .../client/DefaultValidatorStatusProvider.java | 17 +++++++++++------ .../client/ValidatorClientService.java | 4 ++-- .../validator/client/ValidatorStatusLogger.java | 3 +-- .../client/ValidatorStatusProvider.java | 2 ++ ...nnel.java => ValidatorStatusSubscriber.java} | 7 +++---- .../client/ValidatorStatusProviderTest.java | 3 --- 7 files changed, 20 insertions(+), 17 deletions(-) rename validator/client/src/main/java/tech/pegasys/teku/validator/client/{ValidatorStatusesChannel.java => ValidatorStatusSubscriber.java} (71%) diff --git a/validator/client/build.gradle b/validator/client/build.gradle index 3bf44cd4bde..006b812f4fa 100644 --- a/validator/client/build.gradle +++ b/validator/client/build.gradle @@ -26,6 +26,7 @@ dependencies { implementation project(':data:dataexchange') implementation project(':ethereum:signingrecord') implementation project(':infrastructure:bls-keystore') + implementation project(':infrastructure:subscribers') implementation 'org.apache.tuweni:tuweni-bytes' implementation 'commons-io:commons-io' diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusProvider.java index d89baf14a66..27fbf15a8b8 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusProvider.java @@ -32,6 +32,7 @@ import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.metrics.SettableLabelledGauge; import tech.pegasys.teku.infrastructure.metrics.TekuMetricCategory; +import tech.pegasys.teku.infrastructure.subscribers.Subscribers; import tech.pegasys.teku.validator.api.ValidatorApiChannel; import tech.pegasys.teku.validator.client.loader.OwnedValidators; @@ -42,23 +43,23 @@ public class DefaultValidatorStatusProvider implements ValidatorStatusProvider { private final OwnedValidators validators; private final ValidatorApiChannel validatorApiChannel; - private final ValidatorStatusesChannel validatorStatusesChannel; private final AtomicReference> latestValidatorStatuses = new AtomicReference<>(); private final AsyncRunner asyncRunner; private final AtomicBoolean startupComplete = new AtomicBoolean(false); private final SettableLabelledGauge localValidatorCounts; + private final Subscribers validatorStatusSubscribers = + Subscribers.create(true); + public DefaultValidatorStatusProvider( final MetricsSystem metricsSystem, final OwnedValidators validators, final ValidatorApiChannel validatorApiChannel, - final ValidatorStatusesChannel validatorStatusesChannel, final AsyncRunner asyncRunner) { this.validators = validators; this.validatorApiChannel = validatorApiChannel; this.asyncRunner = asyncRunner; - this.validatorStatusesChannel = validatorStatusesChannel; this.localValidatorCounts = SettableLabelledGauge.create( metricsSystem, @@ -68,6 +69,11 @@ public DefaultValidatorStatusProvider( "status"); } + @Override + public void subscribeNewValidatorStatuses(final ValidatorStatusSubscriber subscriber) { + validatorStatusSubscribers.subscribe(subscriber); + } + @Override public SafeFuture initValidatorStatuses() { if (validators.hasNoValidators()) { @@ -86,9 +92,8 @@ public SafeFuture initValidatorStatuses() { final Map validatorStatuses = maybeValidatorStatuses.get(); latestValidatorStatuses.set(validatorStatuses); + validatorStatusSubscribers.forEach(s -> s.onValidatorStatuses(validatorStatuses)); updateValidatorCountMetrics(validatorStatuses); - - validatorStatusesChannel.onNewValidatorStatuses(validatorStatuses); startupComplete.set(true); return SafeFuture.COMPLETE; }) @@ -120,7 +125,7 @@ public void updateValidatorStatuses() { final Map oldValidatorStatuses = latestValidatorStatuses.getAndSet(newValidatorStatuses); - validatorStatusesChannel.onNewValidatorStatuses(newValidatorStatuses); + validatorStatusSubscribers.forEach(s -> s.onValidatorStatuses(newValidatorStatuses)); if (oldValidatorStatuses == null) { return; } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java index 2bce5040376..3329e987241 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java @@ -150,7 +150,6 @@ public static ValidatorClientService create( services.getMetricsSystem(), validatorLoader.getOwnedValidators(), validatorApiChannel, - eventChannels.getPublisher(ValidatorStatusesChannel.class), asyncRunner); final Optional proposerConfigManager; Optional beaconProposerPreparer = Optional.empty(); @@ -467,7 +466,8 @@ private void scheduleValidatorsDuties( } addValidatorCountMetric(metricsSystem, validators); final ValidatorStatusLogger validatorStatusLogger = new ValidatorStatusLogger(validators); - eventChannels.subscribe(ValidatorStatusesChannel.class, validatorStatusLogger); + validatorStatusProvider.subscribeNewValidatorStatuses( + validatorStatusLogger::onNewValidatorStatuses); } public static Path getSlashingProtectionPath(final DataDirLayout dataDirLayout) { diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java index 420c851d107..8cf58d48c1e 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusLogger.java @@ -24,7 +24,7 @@ import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.validator.client.loader.OwnedValidators; -public class ValidatorStatusLogger implements ValidatorStatusesChannel { +public class ValidatorStatusLogger { private static final int VALIDATOR_KEYS_PRINT_LIMIT = 20; final OwnedValidators validators; @@ -35,7 +35,6 @@ public ValidatorStatusLogger(final OwnedValidators validators) { this.validators = validators; } - @Override public void onNewValidatorStatuses( final Map newValidatorStatuses) { final Map oldValidatorStatuses = diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java index 74dc91ed4bb..4478307c8f0 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java @@ -25,4 +25,6 @@ public interface ValidatorStatusProvider { void updateValidatorStatuses(); Optional> getStatuses(); + + void subscribeNewValidatorStatuses(ValidatorStatusSubscriber subscriber); } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusesChannel.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusSubscriber.java similarity index 71% rename from validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusesChannel.java rename to validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusSubscriber.java index d94137490c8..f588cc010a6 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusesChannel.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusSubscriber.java @@ -1,5 +1,5 @@ /* - * Copyright Consensys Software Inc., 2022 + * Copyright Consensys Software Inc., 2023 * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with * the License. You may obtain a copy of the License at @@ -16,8 +16,7 @@ import java.util.Map; import tech.pegasys.teku.api.response.v1.beacon.ValidatorStatus; import tech.pegasys.teku.bls.BLSPublicKey; -import tech.pegasys.teku.infrastructure.events.VoidReturningChannelInterface; -public interface ValidatorStatusesChannel extends VoidReturningChannelInterface { - void onNewValidatorStatuses(Map newValidatorStatuses); +public interface ValidatorStatusSubscriber { + void onValidatorStatuses(Map validatorStatuses); } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java index 3bbc6faeba2..0d589d2c354 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java @@ -46,8 +46,6 @@ public class ValidatorStatusProviderTest { private final Collection validatorKeys = Set.of(validatorKey); private final StubAsyncRunner asyncRunner = new StubAsyncRunner(); private final StubMetricsSystem metricsSystem = new StubMetricsSystem(); - private final ValidatorStatusesChannel validatorStatusesChannel = - mock(ValidatorStatusesChannel.class); private final ValidatorStatusProvider provider = new DefaultValidatorStatusProvider( @@ -55,7 +53,6 @@ public class ValidatorStatusProviderTest { new OwnedValidators( Map.of(validatorKey, new Validator(validatorKey, NO_OP_SIGNER, Optional::empty))), validatorApiChannel, - validatorStatusesChannel, asyncRunner); @Test From 8388e34cee6bbf4eb88adf79cdeb9ac56f2262c8 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Sun, 24 Sep 2023 20:58:51 +0400 Subject: [PATCH 08/18] tmp broken --- ...java => OwnedValidatorStatusProvider.java} | 141 +++++++++++++----- .../client/ValidatorClientService.java | 27 ++-- .../client/ValidatorRegistrator.java | 120 +++++++-------- .../client/ValidatorStatusProvider.java | 12 +- .../client/ValidatorTimingActions.java | 2 + .../client/ValidatorStatusProviderTest.java | 2 +- 6 files changed, 177 insertions(+), 127 deletions(-) rename validator/client/src/main/java/tech/pegasys/teku/validator/client/{DefaultValidatorStatusProvider.java => OwnedValidatorStatusProvider.java} (56%) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java similarity index 56% rename from validator/client/src/main/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusProvider.java rename to validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java index 27fbf15a8b8..6b768f02ced 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/DefaultValidatorStatusProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java @@ -16,8 +16,10 @@ import static tech.pegasys.teku.infrastructure.logging.StatusLogger.STATUS_LOG; import java.time.Duration; +import java.util.HashMap; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; @@ -25,6 +27,7 @@ import java.util.stream.Stream; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.tuweni.bytes.Bytes32; import org.hyperledger.besu.plugin.services.MetricsSystem; import tech.pegasys.teku.api.response.v1.beacon.ValidatorStatus; import tech.pegasys.teku.bls.BLSPublicKey; @@ -33,10 +36,12 @@ import tech.pegasys.teku.infrastructure.metrics.SettableLabelledGauge; import tech.pegasys.teku.infrastructure.metrics.TekuMetricCategory; import tech.pegasys.teku.infrastructure.subscribers.Subscribers; +import tech.pegasys.teku.infrastructure.unsigned.UInt64; +import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.validator.api.ValidatorApiChannel; import tech.pegasys.teku.validator.client.loader.OwnedValidators; -public class DefaultValidatorStatusProvider implements ValidatorStatusProvider { +public class OwnedValidatorStatusProvider implements ValidatorStatusProvider { private static final Logger LOG = LogManager.getLogger(); private static final Duration INITIAL_STATUS_CHECK_RETRY_PERIOD = Duration.ofSeconds(5); @@ -48,11 +53,14 @@ public class DefaultValidatorStatusProvider implements ValidatorStatusProvider { private final AsyncRunner asyncRunner; private final AtomicBoolean startupComplete = new AtomicBoolean(false); private final SettableLabelledGauge localValidatorCounts; + private final AtomicReference lastRunEpoch = new AtomicReference<>(); + private final AtomicReference currentEpoch = new AtomicReference<>(); + private final Spec spec; private final Subscribers validatorStatusSubscribers = Subscribers.create(true); - public DefaultValidatorStatusProvider( + public OwnedValidatorStatusProvider( final MetricsSystem metricsSystem, final OwnedValidators validators, final ValidatorApiChannel validatorApiChannel, @@ -69,13 +77,53 @@ public DefaultValidatorStatusProvider( "status"); } + @Override + public SafeFuture start() { + return initValidatorStatuses(); + } + + @Override + public void onSlot(UInt64 slot) { + final UInt64 epoch = spec.computeEpochAtSlot(slot); + currentEpoch.set(epoch); + final UInt64 firstSlotOfEpoch = spec.computeStartSlotAtEpoch(epoch); + if (slot.equals(firstSlotOfEpoch.plus(1))) { + updateValidatorStatuses(); + } + } + + @Override + public void onHeadUpdate( + UInt64 slot, + Bytes32 previousDutyDependentRoot, + Bytes32 currentDutyDependentRoot, + Bytes32 headBlockRoot) {} + + @Override + public void onPossibleMissedEvents() { + updateValidatorStatuses(); + } + + @Override + public void onValidatorsAdded() { + updateValidatorStatuses(); + } + + @Override + public void onBlockProductionDue(UInt64 slot) {} + + @Override + public void onAttestationCreationDue(UInt64 slot) {} + + @Override + public void onAttestationAggregationDue(UInt64 slot) {} + @Override public void subscribeNewValidatorStatuses(final ValidatorStatusSubscriber subscriber) { validatorStatusSubscribers.subscribe(subscriber); } - @Override - public SafeFuture initValidatorStatuses() { + private SafeFuture initValidatorStatuses() { if (validators.hasNoValidators()) { return SafeFuture.COMPLETE; } @@ -88,12 +136,7 @@ public SafeFuture initValidatorStatuses() { if (maybeValidatorStatuses.isEmpty()) { return retryInitialValidatorStatusCheck(); } - - final Map validatorStatuses = - maybeValidatorStatuses.get(); - latestValidatorStatuses.set(validatorStatuses); - validatorStatusSubscribers.forEach(s -> s.onValidatorStatuses(validatorStatuses)); - updateValidatorCountMetrics(validatorStatuses); + onNewValidatorStatuses(maybeValidatorStatuses.get()); startupComplete.set(true); return SafeFuture.COMPLETE; }) @@ -105,44 +148,68 @@ private SafeFuture retryInitialValidatorStatusCheck() { this::initValidatorStatuses, INITIAL_STATUS_CHECK_RETRY_PERIOD); } - @Override public void updateValidatorStatuses() { if (!startupComplete.get() || validators.hasNoValidators()) { return; } - validatorApiChannel - .getValidatorStatuses(validators.getPublicKeys()) - .thenAccept( - maybeNewValidatorStatuses -> { - if (maybeNewValidatorStatuses.isEmpty()) { - STATUS_LOG.unableToRetrieveValidatorStatusesFromBeaconNode(); - return; - } - - final Map newValidatorStatuses = - maybeNewValidatorStatuses.get(); + if (!needToUpdateAllStatuses()) { + final Set keysToUpdate = + validators.getPublicKeys().stream() + .filter(key -> !latestValidatorStatuses.get().containsKey(key)) + .collect(Collectors.toSet()); + if (keysToUpdate.isEmpty()) { + return; + } + validatorApiChannel + .getValidatorStatuses(keysToUpdate) + .thenAccept( + maybeNewValidatorStatuses -> { + if (maybeNewValidatorStatuses.isEmpty()) { + return; + } + final Map newStatuses = + new HashMap<>(maybeNewValidatorStatuses.get()); + final Map oldStatuses = + Optional.ofNullable(latestValidatorStatuses.get()).orElse(Map.of()); + newStatuses.putAll(oldStatuses); + onNewValidatorStatuses(newStatuses); + }) + .finish(error -> LOG.error("Failed to update validator statuses", error)); + } else { + validatorApiChannel + .getValidatorStatuses(validators.getPublicKeys()) + .thenAccept( + maybeNewValidatorStatuses -> { + if (maybeNewValidatorStatuses.isEmpty()) { + STATUS_LOG.unableToRetrieveValidatorStatusesFromBeaconNode(); + return; + } + onNewValidatorStatuses(maybeNewValidatorStatuses.get()); + }) + .finish(error -> LOG.error("Failed to update validator statuses", error)); + } + } - final Map oldValidatorStatuses = - latestValidatorStatuses.getAndSet(newValidatorStatuses); - validatorStatusSubscribers.forEach(s -> s.onValidatorStatuses(newValidatorStatuses)); - if (oldValidatorStatuses == null) { - return; - } - updateValidatorCountMetrics(newValidatorStatuses); - }) - .finish(error -> LOG.error("Failed to update validator statuses", error)); + private void onNewValidatorStatuses( + final Map newValidatorStatuses) { + latestValidatorStatuses.getAndSet(newValidatorStatuses); + validatorStatusSubscribers.forEach(s -> s.onValidatorStatuses(newValidatorStatuses)); + lastRunEpoch.set(currentEpoch.get()); + updateValidatorCountMetrics(newValidatorStatuses); } - @Override - public Optional> getStatuses() { - return Optional.ofNullable(latestValidatorStatuses.get()); + private boolean needToUpdateAllStatuses() { + if (lastRunEpoch.get() == null) { + return true; + } + return currentEpoch.get().isGreaterThan(lastRunEpoch.get()); } private void updateValidatorCountMetrics( - final Map oldValidatorStatuses) { + final Map newValidatorStatuses) { final Map validatorCountByStatus = - oldValidatorStatuses.values().stream() + newValidatorStatuses.values().stream() .collect(Collectors.groupingBy(Function.identity(), Collectors.counting())); Stream.of(ValidatorStatus.values()) .forEach( @@ -154,6 +221,6 @@ private void updateValidatorCountMetrics( // subset of validators already seen on chain (with status pending*, active_*, exited_* and // withdrawal_*). Unknown validators are calculated by subtraction. localValidatorCounts.set( - validators.getValidatorCount() - oldValidatorStatuses.size(), "unknown"); + validators.getValidatorCount() - newValidatorStatuses.size(), "unknown"); } } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java index 3329e987241..2f1063fd9ed 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java @@ -146,7 +146,7 @@ public static ValidatorClientService create( final ValidatorLoader validatorLoader = createValidatorLoader(services, config, asyncRunner); final ValidatorRestApiConfig validatorApiConfig = config.getValidatorRestApiConfig(); final ValidatorStatusProvider validatorStatusProvider = - new DefaultValidatorStatusProvider( + new OwnedValidatorStatusProvider( services.getMetricsSystem(), validatorLoader.getOwnedValidators(), validatorApiChannel, @@ -181,18 +181,19 @@ public static ValidatorClientService create( Optional.empty(), proposerConfigManager.get(), config.getSpec())); - - validatorRegistrator = - Optional.of( - new ValidatorRegistrator( - config.getSpec(), - validatorLoader.getOwnedValidators(), - validatorStatusProvider, - proposerConfigManager.get(), - new ValidatorRegistrationSigningService( - proposerConfigManager.get(), services.getTimeProvider()), - validatorApiChannel, - validatorConfig.getBuilderRegistrationSendingBatchSize())); + final ValidatorRegistrator validatorRegistratorImpl = + new ValidatorRegistrator( + config.getSpec(), + validatorLoader.getOwnedValidators(), + validatorStatusProvider, + proposerConfigManager.get(), + new ValidatorRegistrationSigningService( + proposerConfigManager.get(), services.getTimeProvider()), + validatorApiChannel, + validatorConfig.getBuilderRegistrationSendingBatchSize()); + validatorStatusProvider.subscribeNewValidatorStatuses( + validatorRegistratorImpl::onNewValidatorStatuses); + validatorRegistrator = Optional.of(validatorRegistratorImpl); } else { proposerConfigManager = Optional.empty(); } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java index 3ff68f8055b..bc411471f4b 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java @@ -56,6 +56,7 @@ public class ValidatorRegistrator implements ValidatorTimingChannel { private final AtomicBoolean firstCallDone = new AtomicBoolean(false); private final AtomicBoolean registrationInProgress = new AtomicBoolean(false); + private final AtomicReference currentEpoch = new AtomicReference<>(); private final AtomicReference lastRunEpoch = new AtomicReference<>(); private final Spec spec; @@ -85,14 +86,8 @@ public ValidatorRegistrator( @Override public void onSlot(final UInt64 slot) { - if (!isReadyToRegister()) { - return; - } - if (registrationNeedsToBeRun(slot)) { - final UInt64 epoch = spec.computeEpochAtSlot(slot); - lastRunEpoch.set(epoch); - registerValidators(); - } + final UInt64 epoch = spec.computeEpochAtSlot(slot); + currentEpoch.set(epoch); } @Override @@ -103,28 +98,10 @@ public void onHeadUpdate( final Bytes32 headBlockRoot) {} @Override - public void onPossibleMissedEvents() { - if (!isReadyToRegister()) { - return; - } - registerValidators(); - } + public void onPossibleMissedEvents() {} @Override - public void onValidatorsAdded() { - // don't execute if the first call hasn't been done yet - if (!isReadyToRegister() || !firstCallDone.get()) { - return; - } - - final List newlyAddedValidators = - ownedValidators.getActiveValidators().stream() - .filter( - validator -> !cachedValidatorRegistrations.containsKey(validator.getPublicKey())) - .toList(); - - registerValidators(newlyAddedValidators).finish(VALIDATOR_LOGGER::registeringValidatorsFailed); - } + public void onValidatorsAdded() {} @Override public void onBlockProductionDue(final UInt64 slot) {} @@ -135,69 +112,84 @@ public void onAttestationCreationDue(final UInt64 slot) {} @Override public void onAttestationAggregationDue(final UInt64 slot) {} + public void onNewValidatorStatuses( + final Map newValidatorStatuses) { + if (!isReadyToRegister()) { + return; + } + final List activeAndPendingValidators = + filterActiveAndPendingValidators(newValidatorStatuses); + if (registrationNeedsToBeRun()) { + registerValidators(activeAndPendingValidators, true); + } else { + final List newValidators = + activeAndPendingValidators.stream() + .filter( + validator -> !cachedValidatorRegistrations.containsKey(validator.getPublicKey())) + .toList(); + if (newValidators.isEmpty()) { + return; + } + registerValidators(newValidators, false); + } + } + public int getNumberOfCachedRegistrations() { return cachedValidatorRegistrations.size(); } private boolean isReadyToRegister() { - if (validatorRegistrationPropertiesProvider.isReadyToProvideProperties() - && validatorStatusProvider.getStatuses().isPresent()) { + if (validatorRegistrationPropertiesProvider.isReadyToProvideProperties()) { return true; } LOG.debug("Not ready to register validator(s)."); return false; } - private boolean registrationNeedsToBeRun(final UInt64 slot) { + private boolean registrationNeedsToBeRun() { final boolean isFirstCall = firstCallDone.compareAndSet(false, true); if (isFirstCall) { return true; } - final UInt64 currentEpoch = spec.computeEpochAtSlot(slot); - final boolean slotIsApplicable = - slot.mod(spec.getSlotsPerEpoch(slot)) - .equals(SLOT_IN_THE_EPOCH_TO_RUN_REGISTRATION.minus(1)); - return slotIsApplicable - && currentEpoch - .minus(lastRunEpoch.get()) - .isGreaterThanOrEqualTo(Constants.EPOCHS_PER_VALIDATOR_REGISTRATION_SUBMISSION); + + return currentEpoch + .get() + .minus(lastRunEpoch.get()) + .isGreaterThanOrEqualTo(Constants.EPOCHS_PER_VALIDATOR_REGISTRATION_SUBMISSION); } - private void registerValidators() { + private void registerValidators( + final List validators, final boolean updateLastRunEpoch) { + if (validators.isEmpty()) { + return; + } if (!registrationInProgress.compareAndSet(false, true)) { - LOG.debug( + LOG.warn( "Validator registration(s) is still in progress. Will skip sending registration(s)."); return; } - final List managedValidators = ownedValidators.getActiveValidators(); - registerValidators(managedValidators) + if (updateLastRunEpoch) { + lastRunEpoch.set(currentEpoch.get()); + } + + validatorRegistrationPropertiesProvider + .refresh() + .thenCompose(__ -> processInBatches(validators)) .handleException(VALIDATOR_LOGGER::registeringValidatorsFailed) .always( () -> { registrationInProgress.set(false); - cleanupCache(managedValidators); + cleanupCache(ownedValidators.getActiveValidators()); }); } - private SafeFuture registerValidators(final List validators) { - if (validators.isEmpty()) { - return SafeFuture.COMPLETE; - } - - return validatorRegistrationPropertiesProvider - .refresh() - .thenCompose(__ -> processInBatches(validators)); - } - private SafeFuture processInBatches(final List validators) { - final List activeAndPendingValidators = filterActiveAndPendingValidators(validators); - if (activeAndPendingValidators.isEmpty()) { + if (validators.isEmpty()) { LOG.info( "All owned validators are either exited or with unknown status. Skipping registration."); return SafeFuture.COMPLETE; } - final List> batchedValidators = - Lists.partition(activeAndPendingValidators, batchSize); + final List> batchedValidators = Lists.partition(validators, batchSize); LOG.debug( "Going to prepare and send {} validator registration(s) to the Beacon Node in {} batch(es)", @@ -240,20 +232,14 @@ private SafeFuture processInBatches(final List validators) { successfullySentRegistrations.get(), validators.size())); } - private List filterActiveAndPendingValidators(final List validators) { + private List filterActiveAndPendingValidators( + final Map statuses) { final Function getKey = validator -> validatorRegistrationPropertiesProvider .getBuilderRegistrationPublicKeyOverride(validator.getPublicKey()) .orElse(validator.getPublicKey()); - final Map statuses = - validatorStatusProvider - .getStatuses() - .orElseThrow( - () -> - new IllegalStateException( - "Expecting loaded validator statuses, but data was not provided")); - return validators.stream() + return ownedValidators.getActiveValidators().stream() .filter( validator -> Optional.ofNullable(statuses.get(getKey.apply(validator))) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java index 4478307c8f0..238b1fe83bd 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorStatusProvider.java @@ -13,18 +13,12 @@ package tech.pegasys.teku.validator.client; -import java.util.Map; -import java.util.Optional; -import tech.pegasys.teku.api.response.v1.beacon.ValidatorStatus; -import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.validator.api.ValidatorTimingChannel; -public interface ValidatorStatusProvider { - SafeFuture initValidatorStatuses(); +public interface ValidatorStatusProvider extends ValidatorTimingChannel { - void updateValidatorStatuses(); - - Optional> getStatuses(); + SafeFuture start(); void subscribeNewValidatorStatuses(ValidatorStatusSubscriber subscriber); } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorTimingActions.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorTimingActions.java index 0a846ce99f8..5ad84ae7637 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorTimingActions.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorTimingActions.java @@ -75,11 +75,13 @@ public void onHeadUpdate( @Override public void onPossibleMissedEvents() { + validatorStatusProvider.updateValidatorStatuses(); delegates.forEach(ValidatorTimingChannel::onPossibleMissedEvents); } @Override public void onValidatorsAdded() { + validatorStatusProvider.updateValidatorStatuses(); delegates.forEach(ValidatorTimingChannel::onValidatorsAdded); } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java index 0d589d2c354..77955761ccd 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorStatusProviderTest.java @@ -48,7 +48,7 @@ public class ValidatorStatusProviderTest { private final StubMetricsSystem metricsSystem = new StubMetricsSystem(); private final ValidatorStatusProvider provider = - new DefaultValidatorStatusProvider( + new OwnedValidatorStatusProvider( metricsSystem, new OwnedValidators( Map.of(validatorKey, new Validator(validatorKey, NO_OP_SIGNER, Optional::empty))), From ccd505e6e52509289997da07c17d4ade2072cf55 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Fri, 29 Sep 2023 09:06:58 +0200 Subject: [PATCH 09/18] Tmp. Mostly working with tests broken --- .../client/OwnedValidatorStatusProvider.java | 28 ++++++++++--------- .../client/ValidatorClientService.java | 5 ++-- .../client/ValidatorRegistrator.java | 4 --- .../client/ValidatorTimingActions.java | 6 ---- 4 files changed, 17 insertions(+), 26 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java index 6b768f02ced..48351c00865 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/OwnedValidatorStatusProvider.java @@ -64,10 +64,12 @@ public OwnedValidatorStatusProvider( final MetricsSystem metricsSystem, final OwnedValidators validators, final ValidatorApiChannel validatorApiChannel, + final Spec spec, final AsyncRunner asyncRunner) { this.validators = validators; this.validatorApiChannel = validatorApiChannel; this.asyncRunner = asyncRunner; + this.spec = spec; this.localValidatorCounts = SettableLabelledGauge.create( metricsSystem, @@ -153,7 +155,19 @@ public void updateValidatorStatuses() { return; } - if (!needToUpdateAllStatuses()) { + if (needToUpdateAllStatuses()) { + validatorApiChannel + .getValidatorStatuses(validators.getPublicKeys()) + .thenAccept( + maybeNewValidatorStatuses -> { + if (maybeNewValidatorStatuses.isEmpty()) { + STATUS_LOG.unableToRetrieveValidatorStatusesFromBeaconNode(); + return; + } + onNewValidatorStatuses(maybeNewValidatorStatuses.get()); + }) + .finish(error -> LOG.error("Failed to update validator statuses", error)); + } else { final Set keysToUpdate = validators.getPublicKeys().stream() .filter(key -> !latestValidatorStatuses.get().containsKey(key)) @@ -176,18 +190,6 @@ public void updateValidatorStatuses() { onNewValidatorStatuses(newStatuses); }) .finish(error -> LOG.error("Failed to update validator statuses", error)); - } else { - validatorApiChannel - .getValidatorStatuses(validators.getPublicKeys()) - .thenAccept( - maybeNewValidatorStatuses -> { - if (maybeNewValidatorStatuses.isEmpty()) { - STATUS_LOG.unableToRetrieveValidatorStatusesFromBeaconNode(); - return; - } - onNewValidatorStatuses(maybeNewValidatorStatuses.get()); - }) - .finish(error -> LOG.error("Failed to update validator statuses", error)); } } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java index 2f1063fd9ed..7fdd2197860 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java @@ -150,6 +150,7 @@ public static ValidatorClientService create( services.getMetricsSystem(), validatorLoader.getOwnedValidators(), validatorApiChannel, + config.getSpec(), asyncRunner); final Optional proposerConfigManager; Optional beaconProposerPreparer = Optional.empty(); @@ -185,7 +186,6 @@ public static ValidatorClientService create( new ValidatorRegistrator( config.getSpec(), validatorLoader.getOwnedValidators(), - validatorStatusProvider, proposerConfigManager.get(), new ValidatorRegistrationSigningService( proposerConfigManager.get(), services.getTimeProvider()), @@ -534,12 +534,11 @@ protected SafeFuture doStart() { eventChannels.subscribe( ValidatorTimingChannel.class, new ValidatorTimingActions( - validatorStatusProvider, validatorIndexProvider, validatorTimingChannels, spec, metricsSystem)); - validatorStatusProvider.initValidatorStatuses().ifExceptionGetsHereRaiseABug(); + validatorStatusProvider.start().ifExceptionGetsHereRaiseABug(); return beaconNodeApi.subscribeToEvents(); }); } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java index bc411471f4b..7779df9a45d 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorRegistrator.java @@ -49,7 +49,6 @@ public class ValidatorRegistrator implements ValidatorTimingChannel { private static final Logger LOG = LogManager.getLogger(); - private static final UInt64 SLOT_IN_THE_EPOCH_TO_RUN_REGISTRATION = UInt64.valueOf(3); private final Map cachedValidatorRegistrations = Maps.newConcurrentMap(); @@ -61,7 +60,6 @@ public class ValidatorRegistrator implements ValidatorTimingChannel { private final Spec spec; private final OwnedValidators ownedValidators; - private final ValidatorStatusProvider validatorStatusProvider; private final ProposerConfigPropertiesProvider validatorRegistrationPropertiesProvider; private final ValidatorRegistrationSigningService validatorRegistrationSigningService; private final ValidatorApiChannel validatorApiChannel; @@ -70,14 +68,12 @@ public class ValidatorRegistrator implements ValidatorTimingChannel { public ValidatorRegistrator( final Spec spec, final OwnedValidators ownedValidators, - final ValidatorStatusProvider validatorStatusProvider, final ProposerConfigPropertiesProvider validatorRegistrationPropertiesProvider, final ValidatorRegistrationSigningService validatorRegistrationSigningService, final ValidatorApiChannel validatorApiChannel, final int batchSize) { this.spec = spec; this.ownedValidators = ownedValidators; - this.validatorStatusProvider = validatorStatusProvider; this.validatorRegistrationPropertiesProvider = validatorRegistrationPropertiesProvider; this.validatorRegistrationSigningService = validatorRegistrationSigningService; this.validatorApiChannel = validatorApiChannel; diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorTimingActions.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorTimingActions.java index 5ad84ae7637..1739ef03a82 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorTimingActions.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorTimingActions.java @@ -25,18 +25,15 @@ public class ValidatorTimingActions implements ValidatorTimingChannel { private final ValidatorIndexProvider validatorIndexProvider; private final Collection delegates; - private final ValidatorStatusProvider validatorStatusProvider; private final Spec spec; private final SettableGauge validatorCurrentEpoch; public ValidatorTimingActions( - final ValidatorStatusProvider validatorStatusProvider, final ValidatorIndexProvider validatorIndexProvider, final Collection delegates, final Spec spec, final MetricsSystem metricsSystem) { - this.validatorStatusProvider = validatorStatusProvider; this.validatorIndexProvider = validatorIndexProvider; this.delegates = delegates; this.spec = spec; @@ -57,7 +54,6 @@ public void onSlot(final UInt64 slot) { final UInt64 firstSlotOfEpoch = spec.computeStartSlotAtEpoch(epoch); if (slot.equals(firstSlotOfEpoch.plus(1))) { validatorIndexProvider.lookupValidators(); - validatorStatusProvider.updateValidatorStatuses(); } } @@ -75,13 +71,11 @@ public void onHeadUpdate( @Override public void onPossibleMissedEvents() { - validatorStatusProvider.updateValidatorStatuses(); delegates.forEach(ValidatorTimingChannel::onPossibleMissedEvents); } @Override public void onValidatorsAdded() { - validatorStatusProvider.updateValidatorStatuses(); delegates.forEach(ValidatorTimingChannel::onValidatorsAdded); } From 76385c6efc552885bb23a27ce9a10510643d61fc Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Fri, 29 Sep 2023 09:25:17 +0200 Subject: [PATCH 10/18] Polishing --- .../java/tech/pegasys/teku/api/ValidatorDataProvider.java | 3 +-- .../tech/pegasys/teku/validator/api/ValidatorApiChannel.java | 4 +++- .../teku/validator/client/ValidatorClientService.java | 5 +---- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java b/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java index 61af75420af..56595665a88 100644 --- a/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java +++ b/data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java @@ -23,7 +23,6 @@ import java.util.Collection; import java.util.List; import java.util.Optional; -import java.util.stream.Collectors; import org.apache.tuweni.bytes.Bytes32; import tech.pegasys.teku.api.exceptions.BadRequestException; import tech.pegasys.teku.api.schema.SignedBeaconBlock; @@ -305,7 +304,7 @@ public SafeFuture registerValidators( .getValidatorStatuses( validatorRegistrations.stream() .map(registration -> registration.getMessage().getPublicKey()) - .collect(Collectors.toList())) + .toList()) .thenComposeChecked( maybeValidatorStatuses -> { if (maybeValidatorStatuses.isEmpty()) { diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorApiChannel.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorApiChannel.java index 03d38f0fd48..19c501e9d85 100644 --- a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorApiChannel.java +++ b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorApiChannel.java @@ -222,7 +222,9 @@ SafeFuture prepareBeaconProposer( /** * Note that only registrations for active or pending validators must be sent to the builder * network. Registrations for unknown or exited validators must be filtered out and not sent to - * the builder network. Expects already filtered input. + * the builder network. + * + *