From c22b71ff4d520827f0a9659b44deba9181ebe163 Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Tue, 17 Oct 2023 08:52:15 +0100 Subject: [PATCH] throw when fee recipient is not configured --- .../SignedValidatorRegistrationFactory.java | 22 ++++---- .../client/ValidatorRegistrator.java | 53 ++++++++++--------- ...ignedValidatorRegistrationFactoryTest.java | 15 +++--- .../client/ValidatorRegistratorTest.java | 2 +- 4 files changed, 49 insertions(+), 43 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/SignedValidatorRegistrationFactory.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/SignedValidatorRegistrationFactory.java index aaba3422b4d..e149a522979 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/SignedValidatorRegistrationFactory.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/SignedValidatorRegistrationFactory.java @@ -49,16 +49,16 @@ public SafeFuture createSignedValidatorRegistration final BLSPublicKey publicKey = validator.getPublicKey(); - final Eth1Address feeRecipient = - proposerConfigPropertiesProvider - .getFeeRecipient(publicKey) - .orElseGet( - () -> { - LOG.warn( - "Fee recipient not configured for {}. Will use a burn address for the validator registration.", - validator.getPublicKey()); - return Eth1Address.ZERO; - }); + final Optional feeRecipient = + proposerConfigPropertiesProvider.getFeeRecipient(publicKey); + + if (feeRecipient.isEmpty()) { + return SafeFuture.failedFuture( + new IllegalStateException( + String.format( + "Fee recipient not configured for %s. Will skip registering it.", publicKey))); + } + final UInt64 gasLimit = proposerConfigPropertiesProvider.getGasLimit(publicKey); final Optional maybeTimestampOverride = @@ -67,7 +67,7 @@ public SafeFuture createSignedValidatorRegistration final ValidatorRegistration validatorRegistration = createValidatorRegistration( VALIDATOR_BUILDER_PUBLIC_KEY.apply(validator, proposerConfigPropertiesProvider), - feeRecipient, + feeRecipient.get(), 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 a757e6c03b5..0be1d608821 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 @@ -123,27 +123,34 @@ public void onAttestationAggregationDue(final UInt64 slot) {} public void onUpdatedValidatorStatuses( final Map newValidatorStatuses, final boolean possibleMissingEvents) { - if (!isReadyToRegister()) { - return; - } - final List validators = getValidatorsRequiringRegistration(newValidatorStatuses); - if (validators.isEmpty()) { - LOG.debug("No validator registrations are required to be sent"); - return; - } - if (registrationNeedsToBeRun(possibleMissingEvents)) { - registerValidators(validators, true); - } else { - final List newValidators = - validators.stream() - .filter( - validator -> !cachedValidatorRegistrations.containsKey(validator.getPublicKey())) - .toList(); - if (newValidators.isEmpty()) { - return; - } - registerValidators(newValidators, false); - } + proposerConfigPropertiesProvider + .refresh() + .thenRun( + () -> { + if (!isReadyToRegister()) { + return; + } + final List validators = + getValidatorsRequiringRegistration(newValidatorStatuses); + if (validators.isEmpty()) { + LOG.debug("No validator registrations are required to be sent"); + return; + } + if (registrationNeedsToBeRun(possibleMissingEvents)) { + registerValidators(validators, true); + } else { + final List newValidators = + validators.stream() + .filter( + validator -> + !cachedValidatorRegistrations.containsKey(validator.getPublicKey())) + .toList(); + if (newValidators.isEmpty()) { + return; + } + registerValidators(newValidators, false); + } + }); } public int getNumberOfCachedRegistrations() { @@ -207,9 +214,7 @@ private void registerValidators( lastRunEpoch.set(currentEpoch.get()); } - proposerConfigPropertiesProvider - .refresh() - .thenCompose(__ -> processInBatches(validators)) + processInBatches(validators) .handleException(VALIDATOR_LOGGER::registeringValidatorsFailed) .always( () -> { diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/SignedValidatorRegistrationFactoryTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/SignedValidatorRegistrationFactoryTest.java index cba1fc9c382..50b2b0325f7 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/SignedValidatorRegistrationFactoryTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/SignedValidatorRegistrationFactoryTest.java @@ -30,6 +30,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.async.SafeFutureAssert; import tech.pegasys.teku.infrastructure.time.StubTimeProvider; import tech.pegasys.teku.infrastructure.time.TimeProvider; import tech.pegasys.teku.infrastructure.unsigned.UInt64; @@ -273,7 +274,7 @@ void setsCorrectFeeRecipientForValidators() { } @TestTemplate - void usesBurnAddressWhenFeeRecipientNotSpecified() { + void throwsWhenFeeRecipientIsNotConfigured() { // no fee recipient provided when(proposerConfigPropertiesProvider.getFeeRecipient(validator.getPublicKey())) .thenReturn(Optional.empty()); @@ -281,12 +282,12 @@ void usesBurnAddressWhenFeeRecipientNotSpecified() { final SafeFuture signedValidatorRegistration = signedValidatorRegistrationFactory.createSignedValidatorRegistration( validator, Optional.empty(), throwable -> {}); - verify(signer).signValidatorRegistration(any()); - assertThat(signedValidatorRegistration) - .isCompletedWithValueMatching( - result -> - result.getMessage().getPublicKey().equals(validator.getPublicKey()) - && result.getMessage().getFeeRecipient().equals(Eth1Address.ZERO)); + verify(signer, never()).signValidatorRegistration(any()); + SafeFutureAssert.assertThatSafeFuture(signedValidatorRegistration) + .isCompletedExceptionallyWithMessage( + String.format( + "Fee recipient not configured for %s. Will skip registering it.", + validator.getPublicKey())); } private SignedValidatorRegistration createSignedValidatorRegistration( 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 f39485128f5..e85bdcbad72 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 @@ -241,7 +241,7 @@ void registersNewlyAddedValidators() { } @TestTemplate - void registerValidatorsEvenIfOneRegistrationSigningFails() { + void registerValidatorsEvenIfOneRegistrationCreationFails() { setOwnedValidators(validator1, validator2, validator3); reset(signedValidatorRegistrationFactory);