From cc1a8ccbd8f864758e3b1eb59b21e9bce5acabda Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Sun, 23 Jan 2022 21:30:05 +0100 Subject: [PATCH 01/14] begin implementing multiple fee recipient --- .../provider/BLSPubKeyKeyDeserializer.java | 26 ++++++++ .../pegasys/teku/provider/JsonProvider.java | 1 + .../cli/options/ValidatorProposerOptions.java | 27 +++++++- .../teku/validator/api/ValidatorConfig.java | 26 +++++++- .../teku/validator/client/ProposerConfig.java | 51 +++++++++++++++ .../client/ProposerConfigService.java | 63 +++++++++++++++++++ .../client/loader/ProposerConfigLoader.java | 53 ++++++++++++++++ .../loader/ProposerConfigLoaderTest.java | 46 ++++++++++++++ .../src/test/resources/proposerConfig.json | 10 +++ 9 files changed, 300 insertions(+), 3 deletions(-) create mode 100644 data/serializer/src/main/java/tech/pegasys/teku/provider/BLSPubKeyKeyDeserializer.java create mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java create mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfigService.java create mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoader.java create mode 100644 validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java create mode 100644 validator/client/src/test/resources/proposerConfig.json diff --git a/data/serializer/src/main/java/tech/pegasys/teku/provider/BLSPubKeyKeyDeserializer.java b/data/serializer/src/main/java/tech/pegasys/teku/provider/BLSPubKeyKeyDeserializer.java new file mode 100644 index 00000000000..673d5d12482 --- /dev/null +++ b/data/serializer/src/main/java/tech/pegasys/teku/provider/BLSPubKeyKeyDeserializer.java @@ -0,0 +1,26 @@ +/* + * Copyright 2022 ConsenSys AG. + * + * 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.provider; + +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.KeyDeserializer; +import tech.pegasys.teku.api.schema.BLSPubKey; + +public class BLSPubKeyKeyDeserializer extends KeyDeserializer { + + @Override + public Object deserializeKey(String key, DeserializationContext ctxt) { + return BLSPubKey.fromHexString(key); + } +} diff --git a/data/serializer/src/main/java/tech/pegasys/teku/provider/JsonProvider.java b/data/serializer/src/main/java/tech/pegasys/teku/provider/JsonProvider.java index a2c16832f2c..8d2b0cb61d9 100644 --- a/data/serializer/src/main/java/tech/pegasys/teku/provider/JsonProvider.java +++ b/data/serializer/src/main/java/tech/pegasys/teku/provider/JsonProvider.java @@ -44,6 +44,7 @@ private void addTekuMappers() { module.addDeserializer(BLSPubKey.class, new BLSPubKeyDeserializer()); module.addDeserializer(BLSSignature.class, new BLSSignatureDeserializer()); module.addSerializer(BLSSignature.class, new BLSSignatureSerializer()); + module.addKeyDeserializer(BLSPubKey.class, new BLSPubKeyKeyDeserializer()); module.addDeserializer(Bytes32.class, new Bytes32Deserializer()); module.addDeserializer(Bytes4.class, new Bytes4Deserializer()); diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java index 614d79d170d..00d55dfeee2 100644 --- a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java +++ b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java @@ -15,6 +15,7 @@ import picocli.CommandLine.Option; import tech.pegasys.teku.config.TekuConfiguration; +import tech.pegasys.teku.validator.api.ValidatorConfig; public class ValidatorProposerOptions { @Option( @@ -26,7 +27,31 @@ public class ValidatorProposerOptions { hidden = true) private String proposerDefaultFeeRecipient = null; + @Option( + names = {"--Xvalidators-proposer-config"}, + paramLabel = "", + description = "remote URL or local file path to load proposer configuration from", + arity = "0..1", + hidden = true) + private String proposerConfig = null; + + @Option( + names = {"--Xvalidators-proposer-config-refresh-rate"}, + paramLabel = "", + description = + "Sets the frequency, in seconds, at which the proposer configuration is reloaded. " + + "0 means never refresh.", + arity = "0..1", + hidden = true) + private long proposerConfigRefreshRate = + ValidatorConfig.DEFAULT_VALIDATOR_PROPOSER_CONFIG_REFRESH_RATE.toSeconds(); + public void configure(TekuConfiguration.Builder builder) { - builder.validator(config -> config.proposerDefaultFeeRecipient(proposerDefaultFeeRecipient)); + builder.validator( + config -> + config + .proposerDefaultFeeRecipient(proposerDefaultFeeRecipient) + .proposerConfigSource(proposerConfig) + .proposerConfigSourceRefreshRate(proposerConfigRefreshRate)); } } diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java index c6f2b5c9de6..a107b3071ab 100644 --- a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java +++ b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java @@ -40,6 +40,8 @@ public class ValidatorConfig { public static final boolean DEFAULT_GENERATE_EARLY_ATTESTATIONS = true; public static final boolean DEFAULT_SEND_ATTESTATIONS_AS_BATCH = true; public static final Optional DEFAULT_GRAFFITI = Optional.empty(); + public static final Duration DEFAULT_VALIDATOR_PROPOSER_CONFIG_REFRESH_RATE = + Duration.ofSeconds(60); private final List validatorKeys; private final List validatorExternalSignerPublicKeySources; @@ -58,6 +60,8 @@ public class ValidatorConfig { private final boolean useDependentRoots; private final boolean generateEarlyAttestations; private final Optional proposerDefaultFeeRecipient; + private final String proposerConfigSource; + private final long proposerConfigSourceRefreshRate; private ValidatorConfig( final List validatorKeys, @@ -76,7 +80,9 @@ private ValidatorConfig( final int validatorExternalSignerConcurrentRequestLimit, final boolean useDependentRoots, final boolean generateEarlyAttestations, - final Optional proposerDefaultFeeRecipient) { + final Optional proposerDefaultFeeRecipient, + final String proposerConfigSource, + final long proposerConfigSourceRefreshRate) { this.validatorKeys = validatorKeys; this.validatorExternalSignerPublicKeySources = validatorExternalSignerPublicKeySources; this.validatorExternalSignerUrl = validatorExternalSignerUrl; @@ -97,6 +103,8 @@ private ValidatorConfig( this.useDependentRoots = useDependentRoots; this.generateEarlyAttestations = generateEarlyAttestations; this.proposerDefaultFeeRecipient = proposerDefaultFeeRecipient; + this.proposerConfigSource = proposerConfigSource; + this.proposerConfigSourceRefreshRate = proposerConfigSourceRefreshRate; } public static Builder builder() { @@ -195,6 +203,8 @@ public static final class Builder { private boolean useDependentRoots = DEFAULT_USE_DEPENDENT_ROOTS; private boolean generateEarlyAttestations = DEFAULT_GENERATE_EARLY_ATTESTATIONS; private Optional proposerDefaultFeeRecipient = Optional.empty(); + private String proposerConfigSource; + private long proposerConfigSourceRefreshRate; private Builder() {} @@ -304,6 +314,16 @@ public Builder proposerDefaultFeeRecipient(final String proposerDefaultFeeRecipi return this; } + public Builder proposerConfigSource(final String proposerConfigSource) { + this.proposerConfigSource = proposerConfigSource; + return this; + } + + public Builder proposerConfigSourceRefreshRate(final long proposerConfigSourceRefreshRate) { + this.proposerConfigSourceRefreshRate = proposerConfigSourceRefreshRate; + return this; + } + public ValidatorConfig build() { validateExternalSignerUrlAndPublicKeys(); validateExternalSignerKeystoreAndPasswordFileConfig(); @@ -326,7 +346,9 @@ public ValidatorConfig build() { validatorExternalSignerConcurrentRequestLimit, useDependentRoots, generateEarlyAttestations, - proposerDefaultFeeRecipient); + proposerDefaultFeeRecipient, + proposerConfigSource, + proposerConfigSourceRefreshRate); } private void validateExternalSignerUrlAndPublicKeys() { diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java new file mode 100644 index 00000000000..7a02cf4521e --- /dev/null +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java @@ -0,0 +1,51 @@ +/* + * Copyright 2022 ConsenSys AG. + * + * 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 com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.Map; +import java.util.Optional; +import tech.pegasys.teku.api.schema.BLSPubKey; +import tech.pegasys.teku.infrastructure.ssz.type.Bytes20; + +public class ProposerConfig { + @JsonProperty(value = "proposer_config", required = true) + private Map proposerConfig; + + @JsonProperty(value = "default_config", required = true) + private Config defaultConfig; + + public Optional getConfigForPubKey(final String pubKey) { + return getConfigForPubKey(BLSPubKey.fromHexString(pubKey)); + } + + public Optional getConfigForPubKey(final BLSPubKey pubKey) { + return Optional.ofNullable(proposerConfig.get(pubKey)); + } + + public Optional getDefaultConfig() { + return Optional.ofNullable(defaultConfig); + } + + @JsonIgnoreProperties(ignoreUnknown = true) + public static class Config { + @JsonProperty(value = "fee_recipient", required = true) + private Bytes20 feeRecipient; + + public Bytes20 getFeeRecipient() { + return feeRecipient; + } + } +} diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfigService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfigService.java new file mode 100644 index 00000000000..c6abed978cb --- /dev/null +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfigService.java @@ -0,0 +1,63 @@ +/* + * Copyright 2022 ConsenSys AG. + * + * 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.time.Duration; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import tech.pegasys.teku.infrastructure.async.AsyncRunner; +import tech.pegasys.teku.infrastructure.async.Cancellable; +import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.service.serviceutils.Service; + +public class ProposerConfigService extends Service { + private static final Logger LOG = LogManager.getLogger(); + static final Duration DEFAULT_REFRESH_RATE = Duration.ofMinutes(1); + + private final Duration refreshRate; + private final AsyncRunner asyncRunner; + private Optional cancellable = Optional.empty(); + private final AtomicBoolean running = new AtomicBoolean(false); + + public ProposerConfigService(final AsyncRunner asyncRunner) { + this.asyncRunner = asyncRunner; + this.refreshRate = DEFAULT_REFRESH_RATE; + } + + @Override + protected SafeFuture doStart() { + cancellable = + Optional.of( + asyncRunner.runWithFixedDelay( + this::loadProposerConfig, + refreshRate, + error -> LOG.error("Failed to refresh proposer configuration", error))); + // Run immediately on start + loadProposerConfig(); + return SafeFuture.COMPLETE; + } + + @Override + protected SafeFuture doStop() { + cancellable.ifPresent(Cancellable::cancel); + cancellable = Optional.empty(); + return SafeFuture.COMPLETE; + } + + private void loadProposerConfig() { + if (running.compareAndSet(false, true)) {} + } +} diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoader.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoader.java new file mode 100644 index 00000000000..b16485bf8a1 --- /dev/null +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoader.java @@ -0,0 +1,53 @@ +/* + * Copyright 2022 ConsenSys AG. + * + * 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.loader; + +import java.io.IOException; +import java.net.URL; +import java.util.Optional; +import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException; +import tech.pegasys.teku.provider.JsonProvider; +import tech.pegasys.teku.validator.client.ProposerConfig; + +public class ProposerConfigLoader { + private final JsonProvider jsonProvider = new JsonProvider(); + + public ProposerConfig getProposerConfig(final String source) { + try { + final ProposerConfig proposerConfig = + jsonProvider.getObjectMapper().readValue(source, ProposerConfig.class); + return proposerConfig; + } catch (IOException ex) { + throw new InvalidConfigurationException("Failed to proposer config from URL " + source, ex); + } + } + + public ProposerConfig getProposerConfig(final URL source) { + try { + final ProposerConfig proposerConfig = + jsonProvider.getObjectMapper().readValue(source, ProposerConfig.class); + return proposerConfig; + } catch (IOException ex) { + throw new InvalidConfigurationException("Failed to proposer config from URL " + source, ex); + } + } + + private Optional getUrl(final String source) { + try { + return Optional.of(new URL(source)); + } catch (IOException e) { + return Optional.empty(); + } + } +} diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java new file mode 100644 index 00000000000..8969684285f --- /dev/null +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java @@ -0,0 +1,46 @@ +/* + * Copyright 2022 ConsenSys AG. + * + * 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.loader; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.google.common.io.Resources; +import java.net.URL; +import java.util.Optional; +import org.junit.jupiter.api.Test; +import tech.pegasys.teku.infrastructure.ssz.type.Bytes20; +import tech.pegasys.teku.validator.client.ProposerConfig; +import tech.pegasys.teku.validator.client.ProposerConfig.Config; + +public class ProposerConfigLoaderTest { + private final ProposerConfigLoader loader = new ProposerConfigLoader(); + + @Test + void shouldLoadValidConfig() { + final URL resource = Resources.getResource("proposerConfig.json"); + + ProposerConfig config = loader.getProposerConfig(resource); + Optional theConfig = + config.getConfigForPubKey( + "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a"); + assertThat(theConfig).isPresent(); + assertThat(theConfig.get().getFeeRecipient()) + .isEqualTo(Bytes20.fromHexString("0x50155530FCE8a85ec7055A5F8b2bE214B3DaeFd3")); + + Optional defaultConfig = config.getDefaultConfig(); + assertThat(defaultConfig).isPresent(); + assertThat(defaultConfig.get().getFeeRecipient()) + .isEqualTo(Bytes20.fromHexString("0x6e35733c5af9B61374A128e6F85f553aF09ff89A")); + } +} diff --git a/validator/client/src/test/resources/proposerConfig.json b/validator/client/src/test/resources/proposerConfig.json new file mode 100644 index 00000000000..28caad57bd3 --- /dev/null +++ b/validator/client/src/test/resources/proposerConfig.json @@ -0,0 +1,10 @@ +{ + "proposer_config": { + "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a": { + "fee_recipient": "0x50155530FCE8a85ec7055A5F8b2bE214B3DaeFd3" + } + }, + "default_config": { + "fee_recipient": "0x6e35733c5af9B61374A128e6F85f553aF09ff89A" + } +} \ No newline at end of file From 844d89cee4ac93a9474e142780370b51062c59f7 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Tue, 25 Jan 2022 16:25:23 +0100 Subject: [PATCH 02/14] big step --- ...lizer.java => Bytes48KeyDeserializer.java} | 6 +- .../pegasys/teku/provider/JsonProvider.java | 4 +- .../cli/options/ValidatorProposerOptions.java | 14 +-- .../teku/validator/api/ValidatorConfig.java | 25 ++++-- .../client/BeaconProposerPreparer.java | 84 +++++++++++++----- .../teku/validator/client/ProposerConfig.java | 25 +++++- .../client/ProposerConfigService.java | 63 -------------- .../client/ValidatorClientService.java | 11 ++- .../client/ValidatorIndexProvider.java | 5 ++ .../AbstractProposerConfigProvider.java | 80 +++++++++++++++++ .../FileProposerConfigProvider.java | 33 +++++++ .../ProposerConfigProvider.java | 43 +++++++++ .../UrlProposerConfigProvider.java | 33 +++++++ .../loader/ProposerConfigLoader.java | 33 ++++--- .../client/BeaconProposerPreparerTest.java | 87 ++++++++++++++----- .../loader/ProposerConfigLoaderTest.java | 1 + 16 files changed, 402 insertions(+), 145 deletions(-) rename data/serializer/src/main/java/tech/pegasys/teku/provider/{BLSPubKeyKeyDeserializer.java => Bytes48KeyDeserializer.java} (84%) delete mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfigService.java create mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java create mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/FileProposerConfigProvider.java create mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProvider.java create mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/UrlProposerConfigProvider.java rename validator/client/src/main/java/tech/pegasys/teku/validator/client/{ => proposerconfig}/loader/ProposerConfigLoader.java (65%) diff --git a/data/serializer/src/main/java/tech/pegasys/teku/provider/BLSPubKeyKeyDeserializer.java b/data/serializer/src/main/java/tech/pegasys/teku/provider/Bytes48KeyDeserializer.java similarity index 84% rename from data/serializer/src/main/java/tech/pegasys/teku/provider/BLSPubKeyKeyDeserializer.java rename to data/serializer/src/main/java/tech/pegasys/teku/provider/Bytes48KeyDeserializer.java index 673d5d12482..0273f1623e6 100644 --- a/data/serializer/src/main/java/tech/pegasys/teku/provider/BLSPubKeyKeyDeserializer.java +++ b/data/serializer/src/main/java/tech/pegasys/teku/provider/Bytes48KeyDeserializer.java @@ -15,12 +15,12 @@ import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.KeyDeserializer; -import tech.pegasys.teku.api.schema.BLSPubKey; +import org.apache.tuweni.bytes.Bytes48; -public class BLSPubKeyKeyDeserializer extends KeyDeserializer { +public class Bytes48KeyDeserializer extends KeyDeserializer { @Override public Object deserializeKey(String key, DeserializationContext ctxt) { - return BLSPubKey.fromHexString(key); + return Bytes48.fromHexString(key); } } diff --git a/data/serializer/src/main/java/tech/pegasys/teku/provider/JsonProvider.java b/data/serializer/src/main/java/tech/pegasys/teku/provider/JsonProvider.java index 8d2b0cb61d9..7d527a85b3d 100644 --- a/data/serializer/src/main/java/tech/pegasys/teku/provider/JsonProvider.java +++ b/data/serializer/src/main/java/tech/pegasys/teku/provider/JsonProvider.java @@ -19,6 +19,7 @@ import com.fasterxml.jackson.databind.module.SimpleModule; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; +import org.apache.tuweni.bytes.Bytes48; import org.apache.tuweni.units.bigints.UInt256; import tech.pegasys.teku.api.response.v1.validator.GetNewBlockResponse; import tech.pegasys.teku.api.response.v2.debug.GetStateResponseV2; @@ -44,7 +45,8 @@ private void addTekuMappers() { module.addDeserializer(BLSPubKey.class, new BLSPubKeyDeserializer()); module.addDeserializer(BLSSignature.class, new BLSSignatureDeserializer()); module.addSerializer(BLSSignature.class, new BLSSignatureSerializer()); - module.addKeyDeserializer(BLSPubKey.class, new BLSPubKeyKeyDeserializer()); + + module.addKeyDeserializer(Bytes48.class, new Bytes48KeyDeserializer()); module.addDeserializer(Bytes32.class, new Bytes32Deserializer()); module.addDeserializer(Bytes4.class, new Bytes4Deserializer()); diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java index 00d55dfeee2..9b7983f2026 100644 --- a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java +++ b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java @@ -36,15 +36,15 @@ public class ValidatorProposerOptions { private String proposerConfig = null; @Option( - names = {"--Xvalidators-proposer-config-refresh-rate"}, - paramLabel = "", + names = {"--Xvalidators-proposer-config-refresh-enabled"}, + paramLabel = "", description = - "Sets the frequency, in seconds, at which the proposer configuration is reloaded. " - + "0 means never refresh.", + "Enable the proposer configuration reload on every proposer preparation (once per epoch)", arity = "0..1", + fallbackValue = "true", hidden = true) - private long proposerConfigRefreshRate = - ValidatorConfig.DEFAULT_VALIDATOR_PROPOSER_CONFIG_REFRESH_RATE.toSeconds(); + private boolean proposerConfigRefreshEnabled = + ValidatorConfig.DEFAULT_VALIDATOR_PROPOSER_CONFIG_REFRESH_ENABLED; public void configure(TekuConfiguration.Builder builder) { builder.validator( @@ -52,6 +52,6 @@ public void configure(TekuConfiguration.Builder builder) { config .proposerDefaultFeeRecipient(proposerDefaultFeeRecipient) .proposerConfigSource(proposerConfig) - .proposerConfigSourceRefreshRate(proposerConfigRefreshRate)); + .refreshProposerConfigFromSource(proposerConfigRefreshEnabled)); } } diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java index a107b3071ab..bf08578a4ec 100644 --- a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java +++ b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java @@ -40,8 +40,7 @@ public class ValidatorConfig { public static final boolean DEFAULT_GENERATE_EARLY_ATTESTATIONS = true; public static final boolean DEFAULT_SEND_ATTESTATIONS_AS_BATCH = true; public static final Optional DEFAULT_GRAFFITI = Optional.empty(); - public static final Duration DEFAULT_VALIDATOR_PROPOSER_CONFIG_REFRESH_RATE = - Duration.ofSeconds(60); + public static final boolean DEFAULT_VALIDATOR_PROPOSER_CONFIG_REFRESH_ENABLED = false; private final List validatorKeys; private final List validatorExternalSignerPublicKeySources; @@ -61,7 +60,7 @@ public class ValidatorConfig { private final boolean generateEarlyAttestations; private final Optional proposerDefaultFeeRecipient; private final String proposerConfigSource; - private final long proposerConfigSourceRefreshRate; + private final boolean refreshProposerConfigFromSource; private ValidatorConfig( final List validatorKeys, @@ -82,7 +81,7 @@ private ValidatorConfig( final boolean generateEarlyAttestations, final Optional proposerDefaultFeeRecipient, final String proposerConfigSource, - final long proposerConfigSourceRefreshRate) { + final boolean refreshProposerConfigFromSource) { this.validatorKeys = validatorKeys; this.validatorExternalSignerPublicKeySources = validatorExternalSignerPublicKeySources; this.validatorExternalSignerUrl = validatorExternalSignerUrl; @@ -104,7 +103,7 @@ private ValidatorConfig( this.generateEarlyAttestations = generateEarlyAttestations; this.proposerDefaultFeeRecipient = proposerDefaultFeeRecipient; this.proposerConfigSource = proposerConfigSource; - this.proposerConfigSourceRefreshRate = proposerConfigSourceRefreshRate; + this.refreshProposerConfigFromSource = refreshProposerConfigFromSource; } public static Builder builder() { @@ -173,6 +172,14 @@ public Optional getProposerDefaultFeeRecipient() { return proposerDefaultFeeRecipient; } + public String getProposerConfigSource() { + return proposerConfigSource; + } + + public boolean getRefreshProposerConfigFromSource() { + return refreshProposerConfigFromSource; + } + private void validateProposerDefaultFeeRecipient() { if (proposerDefaultFeeRecipient.isEmpty() && !(validatorKeys.isEmpty() && validatorExternalSignerPublicKeySources.isEmpty())) { @@ -204,7 +211,7 @@ public static final class Builder { private boolean generateEarlyAttestations = DEFAULT_GENERATE_EARLY_ATTESTATIONS; private Optional proposerDefaultFeeRecipient = Optional.empty(); private String proposerConfigSource; - private long proposerConfigSourceRefreshRate; + private boolean refreshProposerConfigFromSource; private Builder() {} @@ -319,8 +326,8 @@ public Builder proposerConfigSource(final String proposerConfigSource) { return this; } - public Builder proposerConfigSourceRefreshRate(final long proposerConfigSourceRefreshRate) { - this.proposerConfigSourceRefreshRate = proposerConfigSourceRefreshRate; + public Builder refreshProposerConfigFromSource(final boolean refreshProposerConfigFromSource) { + this.refreshProposerConfigFromSource = refreshProposerConfigFromSource; return this; } @@ -348,7 +355,7 @@ public ValidatorConfig build() { generateEarlyAttestations, proposerDefaultFeeRecipient, proposerConfigSource, - proposerConfigSourceRefreshRate); + refreshProposerConfigFromSource); } private void validateExternalSignerUrlAndPublicKeys() { diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java index da4b623842a..6290fe2ca5b 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java @@ -15,9 +15,15 @@ import static tech.pegasys.teku.infrastructure.logging.ValidatorLogger.VALIDATOR_LOGGER; +import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; +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.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException; import tech.pegasys.teku.infrastructure.ssz.type.Bytes20; import tech.pegasys.teku.infrastructure.unsigned.UInt64; @@ -25,26 +31,29 @@ import tech.pegasys.teku.spec.datastructures.operations.versions.bellatrix.BeaconPreparableProposer; import tech.pegasys.teku.validator.api.ValidatorApiChannel; import tech.pegasys.teku.validator.api.ValidatorTimingChannel; -import tech.pegasys.teku.validator.client.loader.OwnedValidators; +import tech.pegasys.teku.validator.client.ProposerConfig.Config; +import tech.pegasys.teku.validator.client.proposerconfig.ProposerConfigProvider; public class BeaconProposerPreparer implements ValidatorTimingChannel { + private static final Logger LOG = LogManager.getLogger(); + private final ValidatorApiChannel validatorApiChannel; private final ValidatorIndexProvider validatorIndexProvider; - private final OwnedValidators validators; + private final ProposerConfigProvider proposerConfigProvider; private final Spec spec; - private final Optional feeRecipient; + private final Optional defaultFeeRecipient; private boolean firstCallDone = false; public BeaconProposerPreparer( ValidatorApiChannel validatorApiChannel, ValidatorIndexProvider validatorIndexProvider, - Optional feeRecipient, - OwnedValidators validators, + ProposerConfigProvider proposerConfigProvider, + Optional defaultFeeRecipient, Spec spec) { this.validatorApiChannel = validatorApiChannel; this.validatorIndexProvider = validatorIndexProvider; - this.validators = validators; - this.feeRecipient = feeRecipient; + this.proposerConfigProvider = proposerConfigProvider; + this.defaultFeeRecipient = defaultFeeRecipient; this.spec = spec; } @@ -52,26 +61,61 @@ public BeaconProposerPreparer( public void onSlot(UInt64 slot) { if (slot.mod(spec.getSlotsPerEpoch(slot)).isZero() || !firstCallDone) { firstCallDone = true; + + SafeFuture> proposerConfigFuture = + proposerConfigProvider.getProposerConfig(); + validatorIndexProvider - .getValidatorIndices(validators.getPublicKeys()) + .getValidatorIndexesByPublicKey() .thenApply( - integers -> - integers.stream() - .map( - index -> - new BeaconPreparableProposer( - UInt64.valueOf(index), getFeeRecipient())) - .collect(Collectors.toList())) + blsPublicKeyIntegerMap -> + proposerConfigFuture + .thenApply( + proposerConfig -> + buildBeaconPreparableProposerList( + proposerConfig, blsPublicKeyIntegerMap)) + .exceptionally( + throwable -> { + LOG.warn( + "An error occurred while obtaining proposer config", throwable); + return buildBeaconPreparableProposerList( + Optional.empty(), blsPublicKeyIntegerMap); + }) + .join()) .thenAccept(validatorApiChannel::prepareBeaconProposer) .finish(VALIDATOR_LOGGER::beaconProposerPreparationFailed); } } - private Bytes20 getFeeRecipient() { - return feeRecipient.orElseThrow( - () -> - new InvalidConfigurationException( - "Invalid configuration. --Xvalidators-proposer-default-fee-recipient must be specified when Bellatrix milestone is active")); + private List buildBeaconPreparableProposerList( + Optional maybeProposerConfig, + Map blsPublicKeyIntegerMap) { + return blsPublicKeyIntegerMap.entrySet().stream() + .map( + blsPublicKeyIntegerEntry -> + new BeaconPreparableProposer( + UInt64.valueOf(blsPublicKeyIntegerEntry.getValue()), + getFeeRecipient(maybeProposerConfig, blsPublicKeyIntegerEntry.getKey()))) + .collect(Collectors.toList()); + } + + private Bytes20 getFeeRecipient( + Optional maybeProposerConfig, BLSPublicKey blsPublicKey) { + return maybeProposerConfig + .flatMap(proposerConfig -> proposerConfig.getConfigForPubKey(blsPublicKey)) + .map(Config::getFeeRecipient) + .orElseGet(() -> getDefaultFeeRecipient(maybeProposerConfig)); + } + + private Bytes20 getDefaultFeeRecipient(Optional maybeProposerConfig) { + return maybeProposerConfig + .flatMap(proposerConfig -> proposerConfig.getDefaultConfig()) + .map(Config::getFeeRecipient) + .or(() -> defaultFeeRecipient) + .orElseThrow( + () -> + new InvalidConfigurationException( + "Invalid configuration. --Xvalidators-proposer-default-fee-recipient must be specified when Bellatrix milestone is active")); } @Override diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java index 7a02cf4521e..8d3de2674b2 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java @@ -13,25 +13,37 @@ package tech.pegasys.teku.validator.client; +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; import java.util.Map; import java.util.Optional; -import tech.pegasys.teku.api.schema.BLSPubKey; +import org.apache.tuweni.bytes.Bytes48; +import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.infrastructure.ssz.type.Bytes20; public class ProposerConfig { @JsonProperty(value = "proposer_config", required = true) - private Map proposerConfig; + private Map proposerConfig; @JsonProperty(value = "default_config", required = true) private Config defaultConfig; + @JsonCreator + ProposerConfig(@JsonProperty(value = "proposer_config", required = true) final Map proposerConfig, @JsonProperty(value = "default_config", required = true) final Config defaultConfig) { + this.proposerConfig = proposerConfig; + this.defaultConfig = defaultConfig; + } + public Optional getConfigForPubKey(final String pubKey) { - return getConfigForPubKey(BLSPubKey.fromHexString(pubKey)); + return getConfigForPubKey(Bytes48.fromHexString(pubKey)); + } + + public Optional getConfigForPubKey(final BLSPublicKey pubKey) { + return getConfigForPubKey(pubKey.toBytesCompressed()); } - public Optional getConfigForPubKey(final BLSPubKey pubKey) { + public Optional getConfigForPubKey(final Bytes48 pubKey) { return Optional.ofNullable(proposerConfig.get(pubKey)); } @@ -44,6 +56,11 @@ public static class Config { @JsonProperty(value = "fee_recipient", required = true) private Bytes20 feeRecipient; + @JsonCreator + Config(@JsonProperty(value = "fee_recipient", required = true) final Bytes20 feeRecipient) { + this.feeRecipient = feeRecipient; + } + public Bytes20 getFeeRecipient() { return feeRecipient; } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfigService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfigService.java deleted file mode 100644 index c6abed978cb..00000000000 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfigService.java +++ /dev/null @@ -1,63 +0,0 @@ -/* - * Copyright 2022 ConsenSys AG. - * - * 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.time.Duration; -import java.util.Optional; -import java.util.concurrent.atomic.AtomicBoolean; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import tech.pegasys.teku.infrastructure.async.AsyncRunner; -import tech.pegasys.teku.infrastructure.async.Cancellable; -import tech.pegasys.teku.infrastructure.async.SafeFuture; -import tech.pegasys.teku.service.serviceutils.Service; - -public class ProposerConfigService extends Service { - private static final Logger LOG = LogManager.getLogger(); - static final Duration DEFAULT_REFRESH_RATE = Duration.ofMinutes(1); - - private final Duration refreshRate; - private final AsyncRunner asyncRunner; - private Optional cancellable = Optional.empty(); - private final AtomicBoolean running = new AtomicBoolean(false); - - public ProposerConfigService(final AsyncRunner asyncRunner) { - this.asyncRunner = asyncRunner; - this.refreshRate = DEFAULT_REFRESH_RATE; - } - - @Override - protected SafeFuture doStart() { - cancellable = - Optional.of( - asyncRunner.runWithFixedDelay( - this::loadProposerConfig, - refreshRate, - error -> LOG.error("Failed to refresh proposer configuration", error))); - // Run immediately on start - loadProposerConfig(); - return SafeFuture.COMPLETE; - } - - @Override - protected SafeFuture doStop() { - cancellable.ifPresent(Cancellable::cancel); - cancellable = Optional.empty(); - return SafeFuture.COMPLETE; - } - - private void loadProposerConfig() { - if (running.compareAndSet(false, true)) {} - } -} 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 ad44fc5a087..68a2b868cd7 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 @@ -48,6 +48,7 @@ import tech.pegasys.teku.validator.client.loader.OwnedValidators; import tech.pegasys.teku.validator.client.loader.PublicKeyLoader; import tech.pegasys.teku.validator.client.loader.ValidatorLoader; +import tech.pegasys.teku.validator.client.proposerconfig.ProposerConfigProvider; import tech.pegasys.teku.validator.client.restapi.ValidatorRestApi; import tech.pegasys.teku.validator.client.restapi.ValidatorRestApiConfig; import tech.pegasys.teku.validator.eventadapter.InProcessBeaconNodeApi; @@ -65,6 +66,7 @@ public class ValidatorClientService extends Service { private final List validatorTimingChannels = new ArrayList<>(); private ValidatorStatusLogger validatorStatusLogger; private ValidatorIndexProvider validatorIndexProvider; + private ProposerConfigProvider proposerConfigProvider; private final SafeFuture initializationComplete = new SafeFuture<>(); @@ -181,6 +183,13 @@ private void initializeValidators( AsyncRunner asyncRunner) { validatorLoader.loadValidators(); final OwnedValidators validators = validatorLoader.getOwnedValidators(); + + this.proposerConfigProvider = + ProposerConfigProvider.create( + asyncRunner, + config.getValidatorConfig().getRefreshProposerConfigFromSource(), + Optional.of(config.getValidatorConfig().getProposerConfigSource())); + this.validatorIndexProvider = new ValidatorIndexProvider(validators, validatorApiChannel, asyncRunner); final BlockDutyFactory blockDutyFactory = @@ -239,8 +248,8 @@ private void initializeValidators( new BeaconProposerPreparer( validatorApiChannel, validatorIndexProvider, + proposerConfigProvider, config.getValidatorConfig().getProposerDefaultFeeRecipient(), - validators, spec)); } addValidatorCountMetric(metricsSystem, validators); diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorIndexProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorIndexProvider.java index b540895b62a..8f7371e1f69 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorIndexProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorIndexProvider.java @@ -13,6 +13,7 @@ package tech.pegasys.teku.validator.client; +import static java.util.Collections.unmodifiableMap; import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toList; @@ -106,4 +107,8 @@ public SafeFuture> getValidatorIndices( __ -> publicKeys.stream().flatMap(key -> getValidatorIndex(key).stream()).collect(toList())); } + + public SafeFuture> getValidatorIndexesByPublicKey() { + return firstSuccessfulRequest.thenApply(__ -> unmodifiableMap(validatorIndexesByPublicKey)); + } } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java new file mode 100644 index 00000000000..66b20f8918d --- /dev/null +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java @@ -0,0 +1,80 @@ +/* + * Copyright 2022 ConsenSys AG. + * + * 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.proposerconfig; + +import java.util.Optional; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import tech.pegasys.teku.infrastructure.async.AsyncRunner; +import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.provider.JsonProvider; +import tech.pegasys.teku.validator.client.ProposerConfig; +import tech.pegasys.teku.validator.client.proposerconfig.loader.ProposerConfigLoader; + +public abstract class AbstractProposerConfigProvider implements ProposerConfigProvider { + private static final Logger LOG = LogManager.getLogger(); + + private final boolean refresh; + private final AsyncRunner asyncRunner; + protected final ProposerConfigLoader proposerConfigLoader = + new ProposerConfigLoader(new JsonProvider().getObjectMapper()); + private Optional lastProposerConfig = Optional.empty(); + private final AtomicBoolean requestInProgress = new AtomicBoolean(false); + + AbstractProposerConfigProvider(final AsyncRunner asyncRunner, final boolean refresh) { + this.asyncRunner = asyncRunner; + this.refresh = refresh; + } + + @Override + public SafeFuture> getProposerConfig() { + if (!requestInProgress.compareAndSet(false, true)) { + if (lastProposerConfig.isPresent()) { + LOG.warn("A proposer config load is already in progress, providing previous config"); + return SafeFuture.completedFuture(lastProposerConfig); + } + return SafeFuture.failedFuture( + new RuntimeException( + "A proposer config load is already in progress and there is no previous config")); + } + + if (lastProposerConfig.isEmpty() || refresh) { + return asyncRunner + .runAsync( + () -> { + lastProposerConfig = Optional.of(internalGetProposerConfig()); + return lastProposerConfig; + }) + .orTimeout(30, TimeUnit.SECONDS) + .exceptionally( + throwable -> { + if (lastProposerConfig.isPresent()) { + LOG.warn("An error occurred while obtaining config, providing previous config"); + return lastProposerConfig; + } + throw new RuntimeException( + "An error occurred while obtaining config and there is no previous config", + throwable); + }) + .alwaysRun(() -> requestInProgress.set(false)); + } + + requestInProgress.set(false); + return SafeFuture.completedFuture(lastProposerConfig); + } + + protected abstract ProposerConfig internalGetProposerConfig(); +} diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/FileProposerConfigProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/FileProposerConfigProvider.java new file mode 100644 index 00000000000..48353144983 --- /dev/null +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/FileProposerConfigProvider.java @@ -0,0 +1,33 @@ +/* + * Copyright 2022 ConsenSys AG. + * + * 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.proposerconfig; + +import java.io.File; +import tech.pegasys.teku.infrastructure.async.AsyncRunner; +import tech.pegasys.teku.validator.client.ProposerConfig; + +public class FileProposerConfigProvider extends AbstractProposerConfigProvider { + private final File source; + + FileProposerConfigProvider( + final AsyncRunner asyncRunner, final boolean refresh, final File source) { + super(asyncRunner, refresh); + this.source = source; + } + + @Override + protected ProposerConfig internalGetProposerConfig() { + return proposerConfigLoader.getProposerConfig(source); + } +} diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProvider.java new file mode 100644 index 00000000000..9301b021b7a --- /dev/null +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProvider.java @@ -0,0 +1,43 @@ +/* + * Copyright 2022 ConsenSys AG. + * + * 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.proposerconfig; + +import java.io.File; +import java.net.MalformedURLException; +import java.net.URL; +import java.util.Optional; +import tech.pegasys.teku.infrastructure.async.AsyncRunner; +import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.validator.client.ProposerConfig; + +public interface ProposerConfigProvider { + ProposerConfigProvider NOOP = () -> SafeFuture.completedFuture(Optional.empty()); + + static ProposerConfigProvider create( + final AsyncRunner asyncRunner, final boolean refresh, final Optional source) { + + if (source.isPresent()) { + try { + URL sourceUrl = new URL(source.get()); + return new UrlProposerConfigProvider(asyncRunner, refresh, sourceUrl); + } catch (MalformedURLException e) { + File sourceFile = new File(source.get()); + return new FileProposerConfigProvider(asyncRunner, refresh, sourceFile); + } + } + return NOOP; + } + + SafeFuture> getProposerConfig(); +} diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/UrlProposerConfigProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/UrlProposerConfigProvider.java new file mode 100644 index 00000000000..29c49e2369e --- /dev/null +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/UrlProposerConfigProvider.java @@ -0,0 +1,33 @@ +/* + * Copyright 2022 ConsenSys AG. + * + * 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.proposerconfig; + +import java.net.URL; +import tech.pegasys.teku.infrastructure.async.AsyncRunner; +import tech.pegasys.teku.validator.client.ProposerConfig; + +public class UrlProposerConfigProvider extends AbstractProposerConfigProvider { + private final URL source; + + UrlProposerConfigProvider( + final AsyncRunner asyncRunner, final boolean refresh, final URL source) { + super(asyncRunner, refresh); + this.source = source; + } + + @Override + protected ProposerConfig internalGetProposerConfig() { + return proposerConfigLoader.getProposerConfig(source); + } +} diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoader.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java similarity index 65% rename from validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoader.java rename to validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java index b16485bf8a1..63a5c3cb32a 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoader.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java @@ -11,43 +11,42 @@ * specific language governing permissions and limitations under the License. */ -package tech.pegasys.teku.validator.client.loader; +package tech.pegasys.teku.validator.client.proposerconfig.loader; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.File; import java.io.IOException; import java.net.URL; -import java.util.Optional; import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException; import tech.pegasys.teku.provider.JsonProvider; import tech.pegasys.teku.validator.client.ProposerConfig; public class ProposerConfigLoader { - private final JsonProvider jsonProvider = new JsonProvider(); + final ObjectMapper objectMapper; - public ProposerConfig getProposerConfig(final String source) { + public ProposerConfigLoader() { + this(new JsonProvider().getObjectMapper()); + } + + public ProposerConfigLoader(final ObjectMapper objectMapper) { + this.objectMapper = objectMapper; + } + + public ProposerConfig getProposerConfig(final File source) { try { - final ProposerConfig proposerConfig = - jsonProvider.getObjectMapper().readValue(source, ProposerConfig.class); + final ProposerConfig proposerConfig = objectMapper.readValue(source, ProposerConfig.class); return proposerConfig; } catch (IOException ex) { - throw new InvalidConfigurationException("Failed to proposer config from URL " + source, ex); + throw new InvalidConfigurationException("Failed to proposer config from File " + source, ex); } } public ProposerConfig getProposerConfig(final URL source) { try { - final ProposerConfig proposerConfig = - jsonProvider.getObjectMapper().readValue(source, ProposerConfig.class); + final ProposerConfig proposerConfig = objectMapper.readValue(source, ProposerConfig.class); return proposerConfig; } catch (IOException ex) { throw new InvalidConfigurationException("Failed to proposer config from URL " + source, ex); } } - - private Optional getUrl(final String source) { - try { - return Optional.of(new URL(source)); - } catch (IOException e) { - return Optional.empty(); - } - } } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/BeaconProposerPreparerTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/BeaconProposerPreparerTest.java index e81aacbaf37..a387722cb9a 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/BeaconProposerPreparerTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/BeaconProposerPreparerTest.java @@ -25,10 +25,10 @@ import java.util.Collection; import java.util.Map; import java.util.Optional; -import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.TestTemplate; import org.mockito.ArgumentCaptor; +import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.core.signatures.Signer; import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.unsigned.UInt64; @@ -38,7 +38,7 @@ import tech.pegasys.teku.spec.datastructures.eth1.Eth1Address; import tech.pegasys.teku.spec.datastructures.operations.versions.bellatrix.BeaconPreparableProposer; import tech.pegasys.teku.validator.api.ValidatorApiChannel; -import tech.pegasys.teku.validator.client.loader.OwnedValidators; +import tech.pegasys.teku.validator.client.proposerconfig.ProposerConfigProvider; @TestSpecContext(milestone = SpecMilestone.BELLATRIX) public class BeaconProposerPreparerTest { @@ -46,8 +46,11 @@ public class BeaconProposerPreparerTest { private final int validator2Index = 23; private final ValidatorIndexProvider validatorIndexProvider = mock(ValidatorIndexProvider.class); private final ValidatorApiChannel validatorApiChannel = mock(ValidatorApiChannel.class); + private final ProposerConfigProvider proposerConfigProvider = mock(ProposerConfigProvider.class); private BeaconProposerPreparer beaconProposerPreparer; - private Eth1Address feeRecipient; + private Eth1Address defaultFeeRecipient; + private Eth1Address defaultFeeRecipientConfig; + private Eth1Address validator1FeeRecipientConfig; private long slotsPerEpoch; @@ -64,40 +67,73 @@ void setUp(SpecContext specContext) { mock(Signer.class), Optional::empty); - Set validatorIndices = Set.of(validator1Index, validator2Index); - OwnedValidators validators = - new OwnedValidators( - Map.of(validator1.getPublicKey(), validator1, validator2.getPublicKey(), validator2)); + Map validatorIndexesByPublicKey = + Map.of( + validator1.getPublicKey(), validator1Index, validator2.getPublicKey(), validator2Index); - feeRecipient = specContext.getDataStructureUtil().randomEth1Address(); + defaultFeeRecipient = specContext.getDataStructureUtil().randomEth1Address(); + defaultFeeRecipientConfig = specContext.getDataStructureUtil().randomEth1Address(); + validator1FeeRecipientConfig = specContext.getDataStructureUtil().randomEth1Address(); + + ProposerConfig proposerConfig = + new ProposerConfig( + Map.of( + validator1.getPublicKey().toBytesCompressed(), + new ProposerConfig.Config(validator1FeeRecipientConfig)), + new ProposerConfig.Config(defaultFeeRecipientConfig)); beaconProposerPreparer = new BeaconProposerPreparer( validatorApiChannel, validatorIndexProvider, - Optional.of(feeRecipient), - validators, + proposerConfigProvider, + Optional.of(defaultFeeRecipient), specContext.getSpec()); slotsPerEpoch = specContext.getSpec().getSlotsPerEpoch(UInt64.ZERO); - when(validatorIndexProvider.getValidatorIndices(validators.getPublicKeys())) - .thenReturn(SafeFuture.completedFuture(validatorIndices)); + when(validatorIndexProvider.getValidatorIndexesByPublicKey()) + .thenReturn(SafeFuture.completedFuture(validatorIndexesByPublicKey)); + when(proposerConfigProvider.getProposerConfig()) + .thenReturn(SafeFuture.completedFuture(Optional.of(proposerConfig))); } @TestTemplate - void should_callPrepareBeaconProposerAtBeginningOfEpoch(SpecContext specContext) { - beaconProposerPreparer.onSlot(UInt64.valueOf(slotsPerEpoch * 2)); + void should_callPrepareBeaconProposerAtBeginningOfEpoch() { + ArgumentCaptor> captor = doCall(); - @SuppressWarnings("unchecked") - final ArgumentCaptor> captor = - ArgumentCaptor.forClass(Collection.class); - verify(validatorApiChannel).prepareBeaconProposer(captor.capture()); + assertThat(captor.getValue()) + .containsExactlyInAnyOrder( + new BeaconPreparableProposer( + UInt64.valueOf(validator1Index), validator1FeeRecipientConfig), + new BeaconPreparableProposer( + UInt64.valueOf(validator2Index), defaultFeeRecipientConfig)); + } + + @TestTemplate + void should_useDefaultFeeRecipientWhenNoConfig() { + when(proposerConfigProvider.getProposerConfig()) + .thenReturn(SafeFuture.completedFuture(Optional.empty())); + + ArgumentCaptor> captor = doCall(); + + assertThat(captor.getValue()) + .containsExactlyInAnyOrder( + new BeaconPreparableProposer(UInt64.valueOf(validator1Index), defaultFeeRecipient), + new BeaconPreparableProposer(UInt64.valueOf(validator2Index), defaultFeeRecipient)); + } + + @TestTemplate + void should_useDefaultFeeRecipientWhenExceptionInConfigProvider() { + when(proposerConfigProvider.getProposerConfig()) + .thenReturn(SafeFuture.failedFuture(new RuntimeException("error"))); + + ArgumentCaptor> captor = doCall(); assertThat(captor.getValue()) .containsExactlyInAnyOrder( - new BeaconPreparableProposer(UInt64.valueOf(validator1Index), feeRecipient), - new BeaconPreparableProposer(UInt64.valueOf(validator2Index), feeRecipient)); + new BeaconPreparableProposer(UInt64.valueOf(validator1Index), defaultFeeRecipient), + new BeaconPreparableProposer(UInt64.valueOf(validator2Index), defaultFeeRecipient)); } @TestTemplate @@ -116,4 +152,15 @@ void should_catchApiExceptions() { beaconProposerPreparer.onSlot(UInt64.ZERO); verify(validatorApiChannel, times(1)).prepareBeaconProposer(any()); } + + private ArgumentCaptor> doCall() { + beaconProposerPreparer.onSlot(UInt64.valueOf(slotsPerEpoch * 2)); + + @SuppressWarnings("unchecked") + final ArgumentCaptor> captor = + ArgumentCaptor.forClass(Collection.class); + verify(validatorApiChannel).prepareBeaconProposer(captor.capture()); + + return captor; + } } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java index 8969684285f..e6408b1767c 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java @@ -22,6 +22,7 @@ import tech.pegasys.teku.infrastructure.ssz.type.Bytes20; import tech.pegasys.teku.validator.client.ProposerConfig; import tech.pegasys.teku.validator.client.ProposerConfig.Config; +import tech.pegasys.teku.validator.client.proposerconfig.loader.ProposerConfigLoader; public class ProposerConfigLoaderTest { private final ProposerConfigLoader loader = new ProposerConfigLoader(); From 15f02a993b970d37eaa98ad9acad4586809deee9 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Tue, 25 Jan 2022 16:31:21 +0100 Subject: [PATCH 03/14] spotlessly --- .../tech/pegasys/teku/validator/client/ProposerConfig.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java index 8d3de2674b2..69e5dce5ce2 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java @@ -30,7 +30,10 @@ public class ProposerConfig { private Config defaultConfig; @JsonCreator - ProposerConfig(@JsonProperty(value = "proposer_config", required = true) final Map proposerConfig, @JsonProperty(value = "default_config", required = true) final Config defaultConfig) { + ProposerConfig( + @JsonProperty(value = "proposer_config", required = true) + final Map proposerConfig, + @JsonProperty(value = "default_config", required = true) final Config defaultConfig) { this.proposerConfig = proposerConfig; this.defaultConfig = defaultConfig; } From f44b83c5dfa425b3289092812350c82bc0297155 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Tue, 25 Jan 2022 18:44:22 +0100 Subject: [PATCH 04/14] config loader tests --- .../teku/provider/Bytes48KeyDeserializer.java | 2 +- .../teku/validator/client/ProposerConfig.java | 18 +++-- .../client/ValidatorClientService.java | 2 +- .../loader/ProposerConfigLoaderTest.java | 72 ++++++++++++++++++- .../resources/proposerConfigInvalid1.json | 10 +++ .../resources/proposerConfigInvalid2.json | 10 +++ .../resources/proposerConfigInvalid3.json | 10 +++ .../resources/proposerConfigInvalid4.json | 9 +++ .../resources/proposerConfigInvalid5.json | 7 ++ ...rConfig.json => proposerConfigValid1.json} | 0 .../test/resources/proposerConfigValid2.json | 7 ++ 11 files changed, 135 insertions(+), 12 deletions(-) create mode 100644 validator/client/src/test/resources/proposerConfigInvalid1.json create mode 100644 validator/client/src/test/resources/proposerConfigInvalid2.json create mode 100644 validator/client/src/test/resources/proposerConfigInvalid3.json create mode 100644 validator/client/src/test/resources/proposerConfigInvalid4.json create mode 100644 validator/client/src/test/resources/proposerConfigInvalid5.json rename validator/client/src/test/resources/{proposerConfig.json => proposerConfigValid1.json} (100%) create mode 100644 validator/client/src/test/resources/proposerConfigValid2.json diff --git a/data/serializer/src/main/java/tech/pegasys/teku/provider/Bytes48KeyDeserializer.java b/data/serializer/src/main/java/tech/pegasys/teku/provider/Bytes48KeyDeserializer.java index 0273f1623e6..b2a8754bd01 100644 --- a/data/serializer/src/main/java/tech/pegasys/teku/provider/Bytes48KeyDeserializer.java +++ b/data/serializer/src/main/java/tech/pegasys/teku/provider/Bytes48KeyDeserializer.java @@ -21,6 +21,6 @@ public class Bytes48KeyDeserializer extends KeyDeserializer { @Override public Object deserializeKey(String key, DeserializationContext ctxt) { - return Bytes48.fromHexString(key); + return Bytes48.fromHexStringStrict(key); } } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java index 69e5dce5ce2..bce77ea971e 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java @@ -13,6 +13,8 @@ package tech.pegasys.teku.validator.client; +import static com.google.common.base.Preconditions.checkNotNull; + import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; @@ -23,17 +25,18 @@ import tech.pegasys.teku.infrastructure.ssz.type.Bytes20; public class ProposerConfig { - @JsonProperty(value = "proposer_config", required = true) + @JsonProperty(value = "proposer_config") private Map proposerConfig; - @JsonProperty(value = "default_config", required = true) + @JsonProperty(value = "default_config") private Config defaultConfig; @JsonCreator ProposerConfig( - @JsonProperty(value = "proposer_config", required = true) - final Map proposerConfig, - @JsonProperty(value = "default_config", required = true) final Config defaultConfig) { + @JsonProperty(value = "proposer_config") final Map proposerConfig, + @JsonProperty(value = "default_config") final Config defaultConfig) { + checkNotNull(proposerConfig, "proposer_config is required"); + checkNotNull(defaultConfig, "default_config is required"); this.proposerConfig = proposerConfig; this.defaultConfig = defaultConfig; } @@ -56,11 +59,12 @@ public Optional getDefaultConfig() { @JsonIgnoreProperties(ignoreUnknown = true) public static class Config { - @JsonProperty(value = "fee_recipient", required = true) + @JsonProperty(value = "fee_recipient") private Bytes20 feeRecipient; @JsonCreator - Config(@JsonProperty(value = "fee_recipient", required = true) final Bytes20 feeRecipient) { + Config(@JsonProperty(value = "fee_recipient") final Bytes20 feeRecipient) { + checkNotNull(feeRecipient, "fee_recipient is required"); this.feeRecipient = feeRecipient; } 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 68a2b868cd7..af9b0fe85b2 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 @@ -188,7 +188,7 @@ private void initializeValidators( ProposerConfigProvider.create( asyncRunner, config.getValidatorConfig().getRefreshProposerConfigFromSource(), - Optional.of(config.getValidatorConfig().getProposerConfigSource())); + Optional.ofNullable(config.getValidatorConfig().getProposerConfigSource())); this.validatorIndexProvider = new ValidatorIndexProvider(validators, validatorApiChannel, asyncRunner); diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java index e6408b1767c..35a8b9fa126 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java @@ -14,8 +14,10 @@ package tech.pegasys.teku.validator.client.loader; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.google.common.io.Resources; +import java.io.File; import java.net.URL; import java.util.Optional; import org.junit.jupiter.api.Test; @@ -28,10 +30,62 @@ public class ProposerConfigLoaderTest { private final ProposerConfigLoader loader = new ProposerConfigLoader(); @Test - void shouldLoadValidConfig() { - final URL resource = Resources.getResource("proposerConfig.json"); + void shouldLoadValidConfigFromUrl() { + final URL resource = Resources.getResource("proposerConfigValid1.json"); - ProposerConfig config = loader.getProposerConfig(resource); + validateContent1(loader.getProposerConfig(resource)); + } + + @Test + void shouldLoadValidConfigFromFile() { + final URL resource = Resources.getResource("proposerConfigValid1.json"); + + validateContent1(loader.getProposerConfig(new File(resource.getPath()))); + } + + @Test + void shouldLoadConfigWithEmptyProposerConfig() { + final URL resource = Resources.getResource("proposerConfigValid2.json"); + + validateContent2(loader.getProposerConfig(resource)); + } + + @Test + void shouldNotLoadInvalidPubKey() { + final URL resource = Resources.getResource("proposerConfigInvalid1.json"); + + assertThatThrownBy(() -> loader.getProposerConfig(resource)); + } + + @Test + void shouldNotLoadNullFeeRecipient() { + final URL resource = Resources.getResource("proposerConfigInvalid2.json"); + + assertThatThrownBy(() -> loader.getProposerConfig(resource)); + } + + @Test + void shouldNotLoadInvalidFeeRecipient() { + final URL resource = Resources.getResource("proposerConfigInvalid2.json"); + + assertThatThrownBy(() -> loader.getProposerConfig(resource)); + } + + @Test + void shouldNotLoadMissingFeeRecipient() { + final URL resource = Resources.getResource("proposerConfigInvalid4.json"); + + assertThatThrownBy(() -> loader.getProposerConfig(resource)); + } + + @Test + void shouldNotLoadMissingDefault() { + final URL resource = Resources.getResource("proposerConfigInvalid5.json"); + + assertThatThrownBy(() -> loader.getProposerConfig(resource)); + } + + private void validateContent1(ProposerConfig config) { Optional theConfig = config.getConfigForPubKey( "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a"); @@ -44,4 +98,16 @@ void shouldLoadValidConfig() { assertThat(defaultConfig.get().getFeeRecipient()) .isEqualTo(Bytes20.fromHexString("0x6e35733c5af9B61374A128e6F85f553aF09ff89A")); } + + private void validateContent2(ProposerConfig config) { + Optional theConfig = + config.getConfigForPubKey( + "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a"); + assertThat(theConfig).isEmpty(); + + Optional defaultConfig = config.getDefaultConfig(); + assertThat(defaultConfig).isPresent(); + assertThat(defaultConfig.get().getFeeRecipient()) + .isEqualTo(Bytes20.fromHexString("0x6e35733c5af9B61374A128e6F85f553aF09ff89A")); + } } diff --git a/validator/client/src/test/resources/proposerConfigInvalid1.json b/validator/client/src/test/resources/proposerConfigInvalid1.json new file mode 100644 index 00000000000..58e2ea9f666 --- /dev/null +++ b/validator/client/src/test/resources/proposerConfigInvalid1.json @@ -0,0 +1,10 @@ +{ + "proposer_config": { + "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd": { + "fee_recipient": "0x50155530FCE8a85ec7055A5F8b2bE214B3DaeFd3" + } + }, + "default_config": { + "fee_recipient": "0x6e35733c5af9B61374A128e6F85f553aF09ff89A" + } +} \ No newline at end of file diff --git a/validator/client/src/test/resources/proposerConfigInvalid2.json b/validator/client/src/test/resources/proposerConfigInvalid2.json new file mode 100644 index 00000000000..9ffef435310 --- /dev/null +++ b/validator/client/src/test/resources/proposerConfigInvalid2.json @@ -0,0 +1,10 @@ +{ + "proposer_config": { + "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a": { + "fee_recipient": null + } + }, + "default_config": { + "fee_recipient": "0x6e35733c5af9B61374A128e6F85f553aF09ff89A" + } +} \ No newline at end of file diff --git a/validator/client/src/test/resources/proposerConfigInvalid3.json b/validator/client/src/test/resources/proposerConfigInvalid3.json new file mode 100644 index 00000000000..665e07a2acd --- /dev/null +++ b/validator/client/src/test/resources/proposerConfigInvalid3.json @@ -0,0 +1,10 @@ +{ + "proposer_config": { + "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a": { + "fee_recipient": "0x50155530FCE8a85ec7055A5F8b2bE214B3DaeF" + } + }, + "default_config": { + "fee_recipient": "0x6e35733c5af9B61374A128e6F85f553aF09ff89A" + } +} \ No newline at end of file diff --git a/validator/client/src/test/resources/proposerConfigInvalid4.json b/validator/client/src/test/resources/proposerConfigInvalid4.json new file mode 100644 index 00000000000..a7f21253019 --- /dev/null +++ b/validator/client/src/test/resources/proposerConfigInvalid4.json @@ -0,0 +1,9 @@ +{ + "proposer_config": { + "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a": { + "fee_recipient": "0x50155530FCE8a85ec7055A5F8b2bE214B3DaeFd3" + } + }, + "default_config": { + } +} \ No newline at end of file diff --git a/validator/client/src/test/resources/proposerConfigInvalid5.json b/validator/client/src/test/resources/proposerConfigInvalid5.json new file mode 100644 index 00000000000..26c2adbcdf7 --- /dev/null +++ b/validator/client/src/test/resources/proposerConfigInvalid5.json @@ -0,0 +1,7 @@ +{ + "proposer_config": { + "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a": { + "fee_recipient": "0x50155530FCE8a85ec7055A5F8b2bE214B3DaeFd3" + } + } +} \ No newline at end of file diff --git a/validator/client/src/test/resources/proposerConfig.json b/validator/client/src/test/resources/proposerConfigValid1.json similarity index 100% rename from validator/client/src/test/resources/proposerConfig.json rename to validator/client/src/test/resources/proposerConfigValid1.json diff --git a/validator/client/src/test/resources/proposerConfigValid2.json b/validator/client/src/test/resources/proposerConfigValid2.json new file mode 100644 index 00000000000..d39b0ba59a8 --- /dev/null +++ b/validator/client/src/test/resources/proposerConfigValid2.json @@ -0,0 +1,7 @@ +{ + "proposer_config": { + }, + "default_config": { + "fee_recipient": "0x6e35733c5af9B61374A128e6F85f553aF09ff89A" + } +} \ No newline at end of file From c4c494f45e561c93113a35eafd4c4326682ebe1d Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Tue, 25 Jan 2022 19:45:37 +0100 Subject: [PATCH 05/14] allow config without proposer_config --- .../teku/validator/client/ProposerConfig.java | 4 ++-- .../AbstractProposerConfigProvider.java | 9 +++++---- .../AbstractProposerConfigProviderTest.java | 16 ++++++++++++++++ .../loader/ProposerConfigLoaderTest.java | 3 +-- .../src/test/resources/proposerConfigValid1.json | 3 ++- .../src/test/resources/proposerConfigValid2.json | 5 ++--- 6 files changed, 28 insertions(+), 12 deletions(-) create mode 100644 validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProviderTest.java rename validator/client/src/test/java/tech/pegasys/teku/validator/client/{ => proposerconfig}/loader/ProposerConfigLoaderTest.java (96%) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java index bce77ea971e..d664b844b78 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java @@ -18,6 +18,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.collect.ImmutableMap; import java.util.Map; import java.util.Optional; import org.apache.tuweni.bytes.Bytes48; @@ -35,9 +36,8 @@ public class ProposerConfig { ProposerConfig( @JsonProperty(value = "proposer_config") final Map proposerConfig, @JsonProperty(value = "default_config") final Config defaultConfig) { - checkNotNull(proposerConfig, "proposer_config is required"); checkNotNull(defaultConfig, "default_config is required"); - this.proposerConfig = proposerConfig; + this.proposerConfig = proposerConfig == null ? ImmutableMap.of() : proposerConfig; this.defaultConfig = defaultConfig; } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java index 66b20f8918d..f1a4626e354 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java @@ -43,12 +43,12 @@ public abstract class AbstractProposerConfigProvider implements ProposerConfigPr public SafeFuture> getProposerConfig() { if (!requestInProgress.compareAndSet(false, true)) { if (lastProposerConfig.isPresent()) { - LOG.warn("A proposer config load is already in progress, providing previous config"); + LOG.warn("A proposer config load is in progress, providing last loaded config"); return SafeFuture.completedFuture(lastProposerConfig); } return SafeFuture.failedFuture( new RuntimeException( - "A proposer config load is already in progress and there is no previous config")); + "A proposer config load is in progress and there is no previously loaded config")); } if (lastProposerConfig.isEmpty() || refresh) { @@ -62,11 +62,12 @@ public SafeFuture> getProposerConfig() { .exceptionally( throwable -> { if (lastProposerConfig.isPresent()) { - LOG.warn("An error occurred while obtaining config, providing previous config"); + LOG.warn( + "An error occurred while obtaining config, providing last loaded config"); return lastProposerConfig; } throw new RuntimeException( - "An error occurred while obtaining config and there is no previous config", + "An error occurred while obtaining config and there is no previously loaded config", throwable); }) .alwaysRun(() -> requestInProgress.set(false)); diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProviderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProviderTest.java new file mode 100644 index 00000000000..4509c88cf03 --- /dev/null +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProviderTest.java @@ -0,0 +1,16 @@ +/* + * Copyright 2022 ConsenSys AG. + * + * 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.proposerconfig; + +public class AbstractProposerConfigProviderTest {} diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoaderTest.java similarity index 96% rename from validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java rename to validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoaderTest.java index 35a8b9fa126..0db33ecefa9 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ProposerConfigLoaderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoaderTest.java @@ -11,7 +11,7 @@ * specific language governing permissions and limitations under the License. */ -package tech.pegasys.teku.validator.client.loader; +package tech.pegasys.teku.validator.client.proposerconfig.loader; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -24,7 +24,6 @@ import tech.pegasys.teku.infrastructure.ssz.type.Bytes20; import tech.pegasys.teku.validator.client.ProposerConfig; import tech.pegasys.teku.validator.client.ProposerConfig.Config; -import tech.pegasys.teku.validator.client.proposerconfig.loader.ProposerConfigLoader; public class ProposerConfigLoaderTest { private final ProposerConfigLoader loader = new ProposerConfigLoader(); diff --git a/validator/client/src/test/resources/proposerConfigValid1.json b/validator/client/src/test/resources/proposerConfigValid1.json index 28caad57bd3..902c4c2c805 100644 --- a/validator/client/src/test/resources/proposerConfigValid1.json +++ b/validator/client/src/test/resources/proposerConfigValid1.json @@ -1,7 +1,8 @@ { "proposer_config": { "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a": { - "fee_recipient": "0x50155530FCE8a85ec7055A5F8b2bE214B3DaeFd3" + "fee_recipient": "0x50155530FCE8a85ec7055A5F8b2bE214B3DaeFd3", + "future_param": "future value" } }, "default_config": { diff --git a/validator/client/src/test/resources/proposerConfigValid2.json b/validator/client/src/test/resources/proposerConfigValid2.json index d39b0ba59a8..7338aeb662c 100644 --- a/validator/client/src/test/resources/proposerConfigValid2.json +++ b/validator/client/src/test/resources/proposerConfigValid2.json @@ -1,7 +1,6 @@ { - "proposer_config": { - }, "default_config": { - "fee_recipient": "0x6e35733c5af9B61374A128e6F85f553aF09ff89A" + "fee_recipient": "0x6e35733c5af9B61374A128e6F85f553aF09ff89A", + "future_param": "future value" } } \ No newline at end of file From 4812ed79e14bd31b8cefdc43d4aa6a236265eb31 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 27 Jan 2022 14:56:03 +0100 Subject: [PATCH 06/14] better initialization unit tests on AbstractProposerConfigProvider --- .../cli/options/ValidatorProposerOptions.java | 4 +- .../teku/validator/api/ValidatorConfig.java | 21 +-- .../validator/api/ValidatorConfigTest.java | 65 ++++---- .../teku/validator/client/ProposerConfig.java | 32 +++- .../client/ValidatorClientService.java | 39 +++-- .../AbstractProposerConfigProvider.java | 54 +++---- .../FileProposerConfigProvider.java | 8 +- .../ProposerConfigProvider.java | 11 +- .../UrlProposerConfigProvider.java | 8 +- .../AbstractProposerConfigProviderTest.java | 16 -- .../ProposerConfigProviderTest.java | 151 ++++++++++++++++++ 11 files changed, 301 insertions(+), 108 deletions(-) delete mode 100644 validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProviderTest.java create mode 100644 validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProviderTest.java diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java index 9b7983f2026..6cf9b686da6 100644 --- a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java +++ b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java @@ -15,7 +15,6 @@ import picocli.CommandLine.Option; import tech.pegasys.teku.config.TekuConfiguration; -import tech.pegasys.teku.validator.api.ValidatorConfig; public class ValidatorProposerOptions { @Option( @@ -43,8 +42,7 @@ public class ValidatorProposerOptions { arity = "0..1", fallbackValue = "true", hidden = true) - private boolean proposerConfigRefreshEnabled = - ValidatorConfig.DEFAULT_VALIDATOR_PROPOSER_CONFIG_REFRESH_ENABLED; + private Boolean proposerConfigRefreshEnabled = null; public void configure(TekuConfiguration.Builder builder) { builder.validator( diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java index bf08578a4ec..e37729b212d 100644 --- a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java +++ b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java @@ -59,7 +59,7 @@ public class ValidatorConfig { private final boolean useDependentRoots; private final boolean generateEarlyAttestations; private final Optional proposerDefaultFeeRecipient; - private final String proposerConfigSource; + private final Optional proposerConfigSource; private final boolean refreshProposerConfigFromSource; private ValidatorConfig( @@ -80,7 +80,7 @@ private ValidatorConfig( final boolean useDependentRoots, final boolean generateEarlyAttestations, final Optional proposerDefaultFeeRecipient, - final String proposerConfigSource, + final Optional proposerConfigSource, final boolean refreshProposerConfigFromSource) { this.validatorKeys = validatorKeys; this.validatorExternalSignerPublicKeySources = validatorExternalSignerPublicKeySources; @@ -168,11 +168,12 @@ public boolean useDependentRoots() { } public Optional getProposerDefaultFeeRecipient() { - validateProposerDefaultFeeRecipient(); + validateProposerDefaultFeeRecipientOrProposerConfigSource(); return proposerDefaultFeeRecipient; } - public String getProposerConfigSource() { + public Optional getProposerConfigSource() { + validateProposerDefaultFeeRecipientOrProposerConfigSource(); return proposerConfigSource; } @@ -180,11 +181,12 @@ public boolean getRefreshProposerConfigFromSource() { return refreshProposerConfigFromSource; } - private void validateProposerDefaultFeeRecipient() { + private void validateProposerDefaultFeeRecipientOrProposerConfigSource() { if (proposerDefaultFeeRecipient.isEmpty() + && proposerConfigSource.isEmpty() && !(validatorKeys.isEmpty() && validatorExternalSignerPublicKeySources.isEmpty())) { throw new InvalidConfigurationException( - "Invalid configuration. --Xvalidators-proposer-default-fee-recipient must be specified when Bellatrix milestone is active"); + "Invalid configuration. --Xvalidators-proposer-default-fee-recipient or --Xvalidators-proposer-config must be specified when Bellatrix milestone is active"); } } @@ -210,8 +212,9 @@ public static final class Builder { private boolean useDependentRoots = DEFAULT_USE_DEPENDENT_ROOTS; private boolean generateEarlyAttestations = DEFAULT_GENERATE_EARLY_ATTESTATIONS; private Optional proposerDefaultFeeRecipient = Optional.empty(); - private String proposerConfigSource; - private boolean refreshProposerConfigFromSource; + private Optional proposerConfigSource = Optional.empty(); + private boolean refreshProposerConfigFromSource = + DEFAULT_VALIDATOR_PROPOSER_CONFIG_REFRESH_ENABLED; private Builder() {} @@ -322,7 +325,7 @@ public Builder proposerDefaultFeeRecipient(final String proposerDefaultFeeRecipi } public Builder proposerConfigSource(final String proposerConfigSource) { - this.proposerConfigSource = proposerConfigSource; + this.proposerConfigSource = Optional.ofNullable(proposerConfigSource); return this; } diff --git a/validator/api/src/test/java/tech/pegasys/teku/validator/api/ValidatorConfigTest.java b/validator/api/src/test/java/tech/pegasys/teku/validator/api/ValidatorConfigTest.java index 364fd8166a8..b7b6f3c9df1 100644 --- a/validator/api/src/test/java/tech/pegasys/teku/validator/api/ValidatorConfigTest.java +++ b/validator/api/src/test/java/tech/pegasys/teku/validator/api/ValidatorConfigTest.java @@ -21,14 +21,13 @@ import org.junit.jupiter.api.Test; import tech.pegasys.teku.bls.BLSTestUtil; import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException; -import tech.pegasys.teku.spec.datastructures.eth1.Eth1Address; class ValidatorConfigTest { private final ValidatorConfig.Builder configBuilder = ValidatorConfig.builder(); @Test - public void shouldThrowExceptionIfExternalPublicKeysAreSpecifiedWithoutExternalSignerUrl() { + public void shouldThrowIfExternalPublicKeysAreSpecifiedWithoutExternalSignerUrl() { final ValidatorConfig.Builder builder = configBuilder.validatorExternalSignerPublicKeySources( List.of(BLSTestUtil.randomKeyPair(0).getPublicKey().toString())); @@ -39,7 +38,7 @@ public void shouldThrowExceptionIfExternalPublicKeysAreSpecifiedWithoutExternalS } @Test - public void noExceptionThrownIfExternalSignerUrlIsSpecifiedWithoutExternalPublicKeys() + public void shouldNotThrowIfExternalSignerUrlIsSpecifiedWithoutExternalPublicKeys() throws MalformedURLException { final ValidatorConfig.Builder builder = configBuilder.validatorExternalSignerUrl(URI.create("http://localhost:9000").toURL()); @@ -47,7 +46,7 @@ public void noExceptionThrownIfExternalSignerUrlIsSpecifiedWithoutExternalPublic } @Test - public void noExceptionThrownIfBothExternalSignerUrlAndPublicKeysAreSpecified() + public void shouldNotThrowIfBothExternalSignerUrlAndPublicKeysAreSpecified() throws MalformedURLException { final ValidatorConfig.Builder builder = configBuilder @@ -59,7 +58,7 @@ public void noExceptionThrownIfBothExternalSignerUrlAndPublicKeysAreSpecified() } @Test - public void shouldThrowExceptionIfExternalSignerKeystoreSpecifiedWithoutPasswordFile() { + public void shouldThrowIfExternalSignerKeystoreSpecifiedWithoutPasswordFile() { final ValidatorConfig.Builder builder = configBuilder.validatorExternalSignerKeystore(Path.of("somepath")); Assertions.assertThatExceptionOfType(InvalidConfigurationException.class) @@ -69,7 +68,7 @@ public void shouldThrowExceptionIfExternalSignerKeystoreSpecifiedWithoutPassword } @Test - public void shouldThrowExceptionIfExternalSignerKeystorePasswordFileIsSpecifiedWithoutKeystore() { + public void shouldThrowIfExternalSignerKeystorePasswordFileIsSpecifiedWithoutKeystore() { final ValidatorConfig.Builder builder = configBuilder.validatorExternalSignerKeystorePasswordFile(Path.of("somepath")); Assertions.assertThatExceptionOfType(InvalidConfigurationException.class) @@ -79,7 +78,7 @@ public void shouldThrowExceptionIfExternalSignerKeystorePasswordFileIsSpecifiedW } @Test - public void noExceptionThrownIfBothExternalSignerKeystoreAndPasswordFileAreSpecified() { + public void shouldNotThrowIfBothExternalSignerKeystoreAndPasswordFileAreSpecified() { final ValidatorConfig.Builder builder = configBuilder .validatorExternalSignerKeystore(Path.of("somepath")) @@ -89,7 +88,7 @@ public void noExceptionThrownIfBothExternalSignerKeystoreAndPasswordFileAreSpeci } @Test - public void shouldThrowExceptionIfExternalSignerTruststoreSpecifiedWithoutPasswordFile() { + public void shouldThrowIfExternalSignerTruststoreSpecifiedWithoutPasswordFile() { final ValidatorConfig.Builder builder = configBuilder.validatorExternalSignerTruststore(Path.of("somepath")); Assertions.assertThatExceptionOfType(InvalidConfigurationException.class) @@ -99,8 +98,7 @@ public void shouldThrowExceptionIfExternalSignerTruststoreSpecifiedWithoutPasswo } @Test - public void - shouldThrowExceptionIfExternalSignerTruststorePasswordFileIsSpecifiedWithoutTruststore() { + public void shouldThrowIfExternalSignerTruststorePasswordFileIsSpecifiedWithoutTruststore() { final ValidatorConfig.Builder builder = configBuilder.validatorExternalSignerTruststorePasswordFile(Path.of("somepath")); Assertions.assertThatExceptionOfType(InvalidConfigurationException.class) @@ -110,7 +108,7 @@ public void shouldThrowExceptionIfExternalSignerTruststoreSpecifiedWithoutPasswo } @Test - public void noExceptionThrownIfBothExternalSignerTruststoreAndPasswordFileAreSpecified() { + public void shouldNotThrowIfBothExternalSignerTruststoreAndPasswordFileAreSpecified() { final ValidatorConfig.Builder builder = configBuilder .validatorExternalSignerTruststore(Path.of("somepath")) @@ -120,7 +118,7 @@ public void noExceptionThrownIfBothExternalSignerTruststoreAndPasswordFileAreSpe } @Test - public void bellatrix_shouldThrowExceptionIfExternalSignerPublicKeySourcesIsSpecified() + public void bellatrix_shouldThrowIfExternalSignerPublicKeySourcesIsSpecified() throws MalformedURLException { final ValidatorConfig config = configBuilder @@ -129,24 +127,18 @@ public void bellatrix_shouldThrowExceptionIfExternalSignerPublicKeySourcesIsSpec .validatorExternalSignerUrl(URI.create("http://localhost:9000").toURL()) .build(); - Assertions.assertThatExceptionOfType(InvalidConfigurationException.class) - .isThrownBy(config::getProposerDefaultFeeRecipient) - .withMessageContaining( - "Invalid configuration. --Xvalidators-proposer-default-fee-recipient must be specified when Bellatrix milestone is active"); + verifyProposerConfigOrProposerDefaultFeeRecipientThrow(config); } @Test - public void bellatrix_shouldThrowExceptionIfValidatorKeysAreSpecified() { + public void bellatrix_shouldThrowIfValidatorKeysAreSpecified() { final ValidatorConfig config = configBuilder.validatorKeys(List.of("some string")).build(); - Assertions.assertThatExceptionOfType(InvalidConfigurationException.class) - .isThrownBy(config::getProposerDefaultFeeRecipient) - .withMessageContaining( - "Invalid configuration. --Xvalidators-proposer-default-fee-recipient must be specified when Bellatrix milestone is active"); + verifyProposerConfigOrProposerDefaultFeeRecipientThrow(config); } @Test - public void bellatrix_noExceptionThrownIfIfExternalSignerPublicKeySourcesIsSpecified() + public void bellatrix_shouldNotThrowIfValidationIsActiveAndDefaultFeeRecipientIsSpecified() throws MalformedURLException { final ValidatorConfig config = configBuilder @@ -156,18 +148,37 @@ public void bellatrix_noExceptionThrownIfIfExternalSignerPublicKeySourcesIsSpeci .proposerDefaultFeeRecipient("0x0000000000000000000000000000000000000000") .build(); - Assertions.assertThatCode(config::getProposerDefaultFeeRecipient).doesNotThrowAnyException(); + verifyProposerConfigOrProposerDefaultFeeRecipientNotThrow(config); } @Test - public void bellatrix_noExceptionThrownIfIfValidatorKeysAreSpecified() { + public void bellatrix_shouldNotThrowIfValidationIsActiveAndProposerConfigSourceIsSpecified() + throws MalformedURLException { final ValidatorConfig config = configBuilder - .validatorKeys(List.of("some string")) - .proposerDefaultFeeRecipient( - Eth1Address.fromHexString("0x0000000000000000000000000000000000000000")) + .validatorExternalSignerPublicKeySources( + List.of(BLSTestUtil.randomKeyPair(0).getPublicKey().toString())) + .validatorExternalSignerUrl(URI.create("http://localhost:9000").toURL()) + .proposerConfigSource("some path") .build(); + verifyProposerConfigOrProposerDefaultFeeRecipientNotThrow(config); + } + + void verifyProposerConfigOrProposerDefaultFeeRecipientNotThrow(final ValidatorConfig config) { Assertions.assertThatCode(config::getProposerDefaultFeeRecipient).doesNotThrowAnyException(); + Assertions.assertThatCode(config::getProposerConfigSource).doesNotThrowAnyException(); + } + + void verifyProposerConfigOrProposerDefaultFeeRecipientThrow(final ValidatorConfig config) { + verifyProposerConfigOrProposerDefaultFeeRecipientThrow(config::getProposerDefaultFeeRecipient); + verifyProposerConfigOrProposerDefaultFeeRecipientThrow(config::getProposerConfigSource); + } + + void verifyProposerConfigOrProposerDefaultFeeRecipientThrow(final Runnable task) { + Assertions.assertThatExceptionOfType(InvalidConfigurationException.class) + .isThrownBy(task::run) + .withMessageContaining( + "Invalid configuration. --Xvalidators-proposer-default-fee-recipient or --Xvalidators-proposer-config must be specified when Bellatrix milestone is active"); } } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java index d664b844b78..83246e30048 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ProposerConfig.java @@ -20,6 +20,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.collect.ImmutableMap; import java.util.Map; +import java.util.Objects; import java.util.Optional; import org.apache.tuweni.bytes.Bytes48; import tech.pegasys.teku.bls.BLSPublicKey; @@ -33,7 +34,7 @@ public class ProposerConfig { private Config defaultConfig; @JsonCreator - ProposerConfig( + public ProposerConfig( @JsonProperty(value = "proposer_config") final Map proposerConfig, @JsonProperty(value = "default_config") final Config defaultConfig) { checkNotNull(defaultConfig, "default_config is required"); @@ -57,13 +58,27 @@ public Optional getDefaultConfig() { return Optional.ofNullable(defaultConfig); } + @Override + public boolean equals(final Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + final ProposerConfig that = (ProposerConfig) o; + return Objects.equals(proposerConfig, that.proposerConfig) + && Objects.equals(defaultConfig, that.defaultConfig); + } + + @Override + public int hashCode() { + return Objects.hash(proposerConfig, defaultConfig); + } + @JsonIgnoreProperties(ignoreUnknown = true) public static class Config { @JsonProperty(value = "fee_recipient") private Bytes20 feeRecipient; @JsonCreator - Config(@JsonProperty(value = "fee_recipient") final Bytes20 feeRecipient) { + public Config(@JsonProperty(value = "fee_recipient") final Bytes20 feeRecipient) { checkNotNull(feeRecipient, "fee_recipient is required"); this.feeRecipient = feeRecipient; } @@ -71,5 +86,18 @@ public static class Config { public Bytes20 getFeeRecipient() { return feeRecipient; } + + @Override + public boolean equals(final Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + final Config that = (Config) o; + return Objects.equals(feeRecipient, that.feeRecipient); + } + + @Override + public int hashCode() { + return Objects.hash(feeRecipient); + } } } 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 af9b0fe85b2..0611c0483c2 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 @@ -30,6 +30,7 @@ import tech.pegasys.teku.infrastructure.io.SystemSignalListener; import tech.pegasys.teku.infrastructure.metrics.TekuMetricCategory; import tech.pegasys.teku.infrastructure.restapi.RestApi; +import tech.pegasys.teku.provider.JsonProvider; import tech.pegasys.teku.service.serviceutils.Service; import tech.pegasys.teku.service.serviceutils.ServiceConfig; import tech.pegasys.teku.service.serviceutils.layout.DataDirLayout; @@ -49,6 +50,7 @@ import tech.pegasys.teku.validator.client.loader.PublicKeyLoader; import tech.pegasys.teku.validator.client.loader.ValidatorLoader; import tech.pegasys.teku.validator.client.proposerconfig.ProposerConfigProvider; +import tech.pegasys.teku.validator.client.proposerconfig.loader.ProposerConfigLoader; import tech.pegasys.teku.validator.client.restapi.ValidatorRestApi; import tech.pegasys.teku.validator.client.restapi.ValidatorRestApiConfig; import tech.pegasys.teku.validator.eventadapter.InProcessBeaconNodeApi; @@ -188,7 +190,8 @@ private void initializeValidators( ProposerConfigProvider.create( asyncRunner, config.getValidatorConfig().getRefreshProposerConfigFromSource(), - Optional.ofNullable(config.getValidatorConfig().getProposerConfigSource())); + new ProposerConfigLoader(new JsonProvider().getObjectMapper()), + config.getValidatorConfig().getProposerConfigSource()); this.validatorIndexProvider = new ValidatorIndexProvider(validators, validatorApiChannel, asyncRunner); @@ -289,22 +292,24 @@ private static void addValidatorCountMetric( @Override protected SafeFuture doStart() { - return initializationComplete.thenCompose( - (__) -> { - validatorRestApi.ifPresent(restApi -> restApi.start().reportExceptions()); - SystemSignalListener.registerReloadConfigListener(validatorLoader::loadValidators); - validatorIndexProvider.lookupValidators(); - eventChannels.subscribe( - ValidatorTimingChannel.class, - new ValidatorTimingActions( - validatorStatusLogger, - validatorIndexProvider, - validatorTimingChannels, - spec, - metricsSystem)); - validatorStatusLogger.printInitialValidatorStatuses().reportExceptions(); - return beaconNodeApi.subscribeToEvents(); - }); + return initializationComplete + .thenCompose(__ -> proposerConfigProvider.getProposerConfig()) + .thenCompose( + __ -> { + validatorRestApi.ifPresent(restApi -> restApi.start().reportExceptions()); + SystemSignalListener.registerReloadConfigListener(validatorLoader::loadValidators); + validatorIndexProvider.lookupValidators(); + eventChannels.subscribe( + ValidatorTimingChannel.class, + new ValidatorTimingActions( + validatorStatusLogger, + validatorIndexProvider, + validatorTimingChannels, + spec, + metricsSystem)); + validatorStatusLogger.printInitialValidatorStatuses().reportExceptions(); + return beaconNodeApi.subscribeToEvents(); + }); } @Override diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java index f1a4626e354..fc292666d45 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java @@ -20,7 +20,6 @@ import org.apache.logging.log4j.Logger; import tech.pegasys.teku.infrastructure.async.AsyncRunner; import tech.pegasys.teku.infrastructure.async.SafeFuture; -import tech.pegasys.teku.provider.JsonProvider; import tech.pegasys.teku.validator.client.ProposerConfig; import tech.pegasys.teku.validator.client.proposerconfig.loader.ProposerConfigLoader; @@ -29,18 +28,25 @@ public abstract class AbstractProposerConfigProvider implements ProposerConfigPr private final boolean refresh; private final AsyncRunner asyncRunner; - protected final ProposerConfigLoader proposerConfigLoader = - new ProposerConfigLoader(new JsonProvider().getObjectMapper()); + protected final ProposerConfigLoader proposerConfigLoader; private Optional lastProposerConfig = Optional.empty(); private final AtomicBoolean requestInProgress = new AtomicBoolean(false); - AbstractProposerConfigProvider(final AsyncRunner asyncRunner, final boolean refresh) { + AbstractProposerConfigProvider( + final AsyncRunner asyncRunner, + final boolean refresh, + final ProposerConfigLoader proposerConfigLoader) { this.asyncRunner = asyncRunner; this.refresh = refresh; + this.proposerConfigLoader = proposerConfigLoader; } @Override public SafeFuture> getProposerConfig() { + if (lastProposerConfig.isPresent() && !refresh) { + return SafeFuture.completedFuture(lastProposerConfig); + } + if (!requestInProgress.compareAndSet(false, true)) { if (lastProposerConfig.isPresent()) { LOG.warn("A proposer config load is in progress, providing last loaded config"); @@ -51,30 +57,24 @@ public SafeFuture> getProposerConfig() { "A proposer config load is in progress and there is no previously loaded config")); } - if (lastProposerConfig.isEmpty() || refresh) { - return asyncRunner - .runAsync( - () -> { - lastProposerConfig = Optional.of(internalGetProposerConfig()); + return asyncRunner + .runAsync( + () -> { + lastProposerConfig = Optional.of(internalGetProposerConfig()); + return lastProposerConfig; + }) + .orTimeout(30, TimeUnit.SECONDS) + .exceptionally( + throwable -> { + if (lastProposerConfig.isPresent()) { + LOG.warn("An error occurred while obtaining config, providing last loaded config"); return lastProposerConfig; - }) - .orTimeout(30, TimeUnit.SECONDS) - .exceptionally( - throwable -> { - if (lastProposerConfig.isPresent()) { - LOG.warn( - "An error occurred while obtaining config, providing last loaded config"); - return lastProposerConfig; - } - throw new RuntimeException( - "An error occurred while obtaining config and there is no previously loaded config", - throwable); - }) - .alwaysRun(() -> requestInProgress.set(false)); - } - - requestInProgress.set(false); - return SafeFuture.completedFuture(lastProposerConfig); + } + throw new RuntimeException( + "An error occurred while obtaining config and there is no previously loaded config", + throwable); + }) + .alwaysRun(() -> requestInProgress.set(false)); } protected abstract ProposerConfig internalGetProposerConfig(); diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/FileProposerConfigProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/FileProposerConfigProvider.java index 48353144983..4a22fcf7011 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/FileProposerConfigProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/FileProposerConfigProvider.java @@ -16,13 +16,17 @@ import java.io.File; import tech.pegasys.teku.infrastructure.async.AsyncRunner; import tech.pegasys.teku.validator.client.ProposerConfig; +import tech.pegasys.teku.validator.client.proposerconfig.loader.ProposerConfigLoader; public class FileProposerConfigProvider extends AbstractProposerConfigProvider { private final File source; FileProposerConfigProvider( - final AsyncRunner asyncRunner, final boolean refresh, final File source) { - super(asyncRunner, refresh); + final AsyncRunner asyncRunner, + final boolean refresh, + final ProposerConfigLoader proposerConfigLoader, + final File source) { + super(asyncRunner, refresh, proposerConfigLoader); this.source = source; } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProvider.java index 9301b021b7a..cdc1cdfa715 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProvider.java @@ -20,20 +20,25 @@ import tech.pegasys.teku.infrastructure.async.AsyncRunner; import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.validator.client.ProposerConfig; +import tech.pegasys.teku.validator.client.proposerconfig.loader.ProposerConfigLoader; public interface ProposerConfigProvider { ProposerConfigProvider NOOP = () -> SafeFuture.completedFuture(Optional.empty()); static ProposerConfigProvider create( - final AsyncRunner asyncRunner, final boolean refresh, final Optional source) { + final AsyncRunner asyncRunner, + final boolean refresh, + final ProposerConfigLoader proposerConfigLoader, + final Optional source) { if (source.isPresent()) { try { URL sourceUrl = new URL(source.get()); - return new UrlProposerConfigProvider(asyncRunner, refresh, sourceUrl); + return new UrlProposerConfigProvider(asyncRunner, refresh, proposerConfigLoader, sourceUrl); } catch (MalformedURLException e) { File sourceFile = new File(source.get()); - return new FileProposerConfigProvider(asyncRunner, refresh, sourceFile); + return new FileProposerConfigProvider( + asyncRunner, refresh, proposerConfigLoader, sourceFile); } } return NOOP; diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/UrlProposerConfigProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/UrlProposerConfigProvider.java index 29c49e2369e..1bf70f04c1f 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/UrlProposerConfigProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/UrlProposerConfigProvider.java @@ -16,13 +16,17 @@ import java.net.URL; import tech.pegasys.teku.infrastructure.async.AsyncRunner; import tech.pegasys.teku.validator.client.ProposerConfig; +import tech.pegasys.teku.validator.client.proposerconfig.loader.ProposerConfigLoader; public class UrlProposerConfigProvider extends AbstractProposerConfigProvider { private final URL source; UrlProposerConfigProvider( - final AsyncRunner asyncRunner, final boolean refresh, final URL source) { - super(asyncRunner, refresh); + final AsyncRunner asyncRunner, + final boolean refresh, + final ProposerConfigLoader proposerConfigLoader, + final URL source) { + super(asyncRunner, refresh, proposerConfigLoader); this.source = source; } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProviderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProviderTest.java deleted file mode 100644 index 4509c88cf03..00000000000 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProviderTest.java +++ /dev/null @@ -1,16 +0,0 @@ -/* - * Copyright 2022 ConsenSys AG. - * - * 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.proposerconfig; - -public class AbstractProposerConfigProviderTest {} diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProviderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProviderTest.java new file mode 100644 index 00000000000..dc7db7965d6 --- /dev/null +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProviderTest.java @@ -0,0 +1,151 @@ +/* + * Copyright 2022 ConsenSys AG. + * + * 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.proposerconfig; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.google.common.collect.ImmutableMap; +import java.io.File; +import java.util.Optional; +import org.junit.jupiter.api.Test; +import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.infrastructure.async.StubAsyncRunner; +import tech.pegasys.teku.infrastructure.ssz.type.Bytes20; +import tech.pegasys.teku.validator.client.ProposerConfig; +import tech.pegasys.teku.validator.client.proposerconfig.loader.ProposerConfigLoader; + +public class ProposerConfigProviderTest { + private static final String SOURCE = "some/file/path"; + + private final StubAsyncRunner asyncRunner = new StubAsyncRunner(); + private final ProposerConfigLoader proposerConfigLoader = mock(ProposerConfigLoader.class); + + private final ProposerConfig proposerConfigA = + new ProposerConfig(ImmutableMap.of(), new ProposerConfig.Config(Bytes20.ZERO)); + + private final ProposerConfig proposerConfigB = + new ProposerConfig( + ImmutableMap.of(), + new ProposerConfig.Config( + Bytes20.fromHexString("0x6e35733c5af9B61374A128e6F85f553aF09ff89A"))); + + private final ProposerConfigProvider proposerConfigProvider = + ProposerConfigProvider.create(asyncRunner, true, proposerConfigLoader, Optional.of(SOURCE)); + private final File sourceFile = new File(SOURCE); + + @Test + void getProposerConfig_shouldReturnConfig() { + SafeFuture> futureMaybeConfig = + proposerConfigProvider.getProposerConfig(); + + assertThat(futureMaybeConfig).isNotCompleted(); + + when(proposerConfigLoader.getProposerConfig(sourceFile)).thenReturn(proposerConfigA); + asyncRunner.executeQueuedActions(); + + assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); + } + + @Test + void getProposerConfig_onErrorShouldThrowWhenNoLastConfigAvailable() { + SafeFuture> futureMaybeConfig = + proposerConfigProvider.getProposerConfig(); + + when(proposerConfigLoader.getProposerConfig(sourceFile)) + .thenThrow(new RuntimeException("error")); + asyncRunner.executeQueuedActions(); + + assertThat(futureMaybeConfig).isCompletedExceptionally(); + } + + @Test + void getProposerConfig_onErrorShouldReturnLastConfigWhenLastConfigAvailable() { + SafeFuture> futureMaybeConfig = + proposerConfigProvider.getProposerConfig(); + + when(proposerConfigLoader.getProposerConfig(sourceFile)).thenReturn(proposerConfigA); + asyncRunner.executeQueuedActions(); + + assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); + + futureMaybeConfig = proposerConfigProvider.getProposerConfig(); + + when(proposerConfigLoader.getProposerConfig(sourceFile)) + .thenThrow(new RuntimeException("error")); + asyncRunner.executeQueuedActions(); + + assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); + } + + @Test + void getProposerConfig_onConcurrentCallsShouldThrowWhenNoLastConfigAvailable() { + SafeFuture> futureMaybeConfig = + proposerConfigProvider.getProposerConfig(); + + SafeFuture> futureMaybeConfig2 = + proposerConfigProvider.getProposerConfig(); + assertThat(futureMaybeConfig2).isCompletedExceptionally(); + + when(proposerConfigLoader.getProposerConfig(sourceFile)).thenReturn(proposerConfigA); + asyncRunner.executeQueuedActions(); + + assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); + } + + @Test + void getProposerConfig_onConcurrentCallsShouldReturnLastConfigWhenLastConfigAvailable() { + SafeFuture> futureMaybeConfig = + proposerConfigProvider.getProposerConfig(); + + when(proposerConfigLoader.getProposerConfig(sourceFile)).thenReturn(proposerConfigA); + asyncRunner.executeQueuedActions(); + + assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); + + futureMaybeConfig = proposerConfigProvider.getProposerConfig(); + SafeFuture> futureMaybeConfig2 = + proposerConfigProvider.getProposerConfig(); + + assertThat(futureMaybeConfig2).isCompletedWithValue(Optional.of(proposerConfigA)); + + when(proposerConfigLoader.getProposerConfig(sourceFile)).thenReturn(proposerConfigB); + asyncRunner.executeQueuedActions(); + + assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigB)); + } + + @Test + void getProposerConfig_shouldAlwaysReturnFirstValidConfigWhenRefreshIsFalse() { + final ProposerConfigProvider proposerConfigProvider = + ProposerConfigProvider.create( + asyncRunner, false, proposerConfigLoader, Optional.of(SOURCE)); + + SafeFuture> futureMaybeConfig = + proposerConfigProvider.getProposerConfig(); + + assertThat(futureMaybeConfig).isNotCompleted(); + + when(proposerConfigLoader.getProposerConfig(sourceFile)).thenReturn(proposerConfigA); + asyncRunner.executeQueuedActions(); + + assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); + + when(proposerConfigLoader.getProposerConfig(sourceFile)).thenReturn(proposerConfigB); + + futureMaybeConfig = proposerConfigProvider.getProposerConfig(); + assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); + } +} From afebdb30499232e441929c1ff1a4f0d4479b40ab Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 27 Jan 2022 16:18:34 +0100 Subject: [PATCH 07/14] fix UT --- .../pegasys/teku/cli/options/ValidatorProposerOptions.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java index 6cf9b686da6..9b7983f2026 100644 --- a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java +++ b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java @@ -15,6 +15,7 @@ import picocli.CommandLine.Option; import tech.pegasys.teku.config.TekuConfiguration; +import tech.pegasys.teku.validator.api.ValidatorConfig; public class ValidatorProposerOptions { @Option( @@ -42,7 +43,8 @@ public class ValidatorProposerOptions { arity = "0..1", fallbackValue = "true", hidden = true) - private Boolean proposerConfigRefreshEnabled = null; + private boolean proposerConfigRefreshEnabled = + ValidatorConfig.DEFAULT_VALIDATOR_PROPOSER_CONFIG_REFRESH_ENABLED; public void configure(TekuConfiguration.Builder builder) { builder.validator( From 410a8b231fdcbc6f927459aa196b20e7e2a953f3 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 27 Jan 2022 17:27:50 +0100 Subject: [PATCH 08/14] fix AT --- .../client/ValidatorClientService.java | 27 ++++++++++++------- 1 file changed, 17 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 0611c0483c2..a4178b173c0 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 @@ -68,7 +68,7 @@ public class ValidatorClientService extends Service { private final List validatorTimingChannels = new ArrayList<>(); private ValidatorStatusLogger validatorStatusLogger; private ValidatorIndexProvider validatorIndexProvider; - private ProposerConfigProvider proposerConfigProvider; + private Optional proposerConfigProvider; private final SafeFuture initializationComplete = new SafeFuture<>(); @@ -186,13 +186,6 @@ private void initializeValidators( validatorLoader.loadValidators(); final OwnedValidators validators = validatorLoader.getOwnedValidators(); - this.proposerConfigProvider = - ProposerConfigProvider.create( - asyncRunner, - config.getValidatorConfig().getRefreshProposerConfigFromSource(), - new ProposerConfigLoader(new JsonProvider().getObjectMapper()), - config.getValidatorConfig().getProposerConfigSource()); - this.validatorIndexProvider = new ValidatorIndexProvider(validators, validatorApiChannel, asyncRunner); final BlockDutyFactory blockDutyFactory = @@ -247,13 +240,23 @@ private void initializeValidators( } if (spec.isMilestoneSupported(SpecMilestone.BELLATRIX)) { + proposerConfigProvider = + Optional.of( + ProposerConfigProvider.create( + asyncRunner, + config.getValidatorConfig().getRefreshProposerConfigFromSource(), + new ProposerConfigLoader(new JsonProvider().getObjectMapper()), + config.getValidatorConfig().getProposerConfigSource())); + validatorTimingChannels.add( new BeaconProposerPreparer( validatorApiChannel, validatorIndexProvider, - proposerConfigProvider, + proposerConfigProvider.get(), config.getValidatorConfig().getProposerDefaultFeeRecipient(), spec)); + } else { + proposerConfigProvider = Optional.empty(); } addValidatorCountMetric(metricsSystem, validators); this.validatorStatusLogger = @@ -293,7 +296,11 @@ private static void addValidatorCountMetric( @Override protected SafeFuture doStart() { return initializationComplete - .thenCompose(__ -> proposerConfigProvider.getProposerConfig()) + .thenCompose( + __ -> + proposerConfigProvider + .map(ProposerConfigProvider::getProposerConfig) + .orElse(SafeFuture.completedFuture(Optional.empty()))) .thenCompose( __ -> { validatorRestApi.ifPresent(restApi -> restApi.start().reportExceptions()); From 974ad7b0402ee361b874af676bbf712f5c83a4b9 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 27 Jan 2022 18:06:01 +0100 Subject: [PATCH 09/14] improve error messages --- .../loader/ProposerConfigLoader.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java index 63a5c3cb32a..291be3dba0b 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.net.URL; import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException; +import tech.pegasys.teku.infrastructure.http.UrlSanitizer; import tech.pegasys.teku.provider.JsonProvider; import tech.pegasys.teku.validator.client.ProposerConfig; @@ -34,19 +35,22 @@ public ProposerConfigLoader(final ObjectMapper objectMapper) { public ProposerConfig getProposerConfig(final File source) { try { - final ProposerConfig proposerConfig = objectMapper.readValue(source, ProposerConfig.class); - return proposerConfig; + return objectMapper.readValue(source, ProposerConfig.class); } catch (IOException ex) { - throw new InvalidConfigurationException("Failed to proposer config from File " + source, ex); + throw new InvalidConfigurationException( + "Failed to load proposer config from file: " + source, ex); } } public ProposerConfig getProposerConfig(final URL source) { try { - final ProposerConfig proposerConfig = objectMapper.readValue(source, ProposerConfig.class); - return proposerConfig; + return objectMapper.readValue(source, ProposerConfig.class); } catch (IOException ex) { - throw new InvalidConfigurationException("Failed to proposer config from URL " + source, ex); + + throw new InvalidConfigurationException( + "Failed to load proposer config from URL:" + + UrlSanitizer.sanitizePotentialUrl(source.toString()), + ex); } } } From 84957de816b23691b02cb641ad0c057241c2945a Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 27 Jan 2022 19:59:46 +0100 Subject: [PATCH 10/14] improve error messages again --- .../proposerconfig/AbstractProposerConfigProvider.java | 5 ++++- .../client/proposerconfig/loader/ProposerConfigLoader.java | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java index fc292666d45..fff3663aca7 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java @@ -67,13 +67,16 @@ public SafeFuture> getProposerConfig() { .exceptionally( throwable -> { if (lastProposerConfig.isPresent()) { - LOG.warn("An error occurred while obtaining config, providing last loaded config"); + LOG.warn( + "An error occurred while obtaining config, providing last loaded config", + throwable); return lastProposerConfig; } throw new RuntimeException( "An error occurred while obtaining config and there is no previously loaded config", throwable); }) + .thenPeek(__ -> LOG.info("proposer config successfully loaded")) .alwaysRun(() -> requestInProgress.set(false)); } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java index 291be3dba0b..b50b460c286 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java @@ -48,7 +48,7 @@ public ProposerConfig getProposerConfig(final URL source) { } catch (IOException ex) { throw new InvalidConfigurationException( - "Failed to load proposer config from URL:" + "Failed to load proposer config from URL: " + UrlSanitizer.sanitizePotentialUrl(source.toString()), ex); } From b874703ee954bae9c9a0bcdcb4d41f6aec57d130 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Mon, 31 Jan 2022 10:51:46 +0100 Subject: [PATCH 11/14] better concurrency, simplification of ProposerConfigLoader to be URL only --- .../client/BeaconProposerPreparer.java | 12 ++-- .../AbstractProposerConfigProvider.java | 62 ++++++++++--------- .../FileProposerConfigProvider.java | 37 ----------- .../ProposerConfigProvider.java | 14 +++-- .../loader/ProposerConfigLoader.java | 12 +--- .../ProposerConfigProviderTest.java | 31 ++++++---- .../loader/ProposerConfigLoaderTest.java | 8 --- 7 files changed, 67 insertions(+), 109 deletions(-) delete mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/FileProposerConfigProvider.java diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java index 6290fe2ca5b..0802fe3e329 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java @@ -67,21 +67,19 @@ public void onSlot(UInt64 slot) { validatorIndexProvider .getValidatorIndexesByPublicKey() - .thenApply( - blsPublicKeyIntegerMap -> + .thenCompose( + publicKeyToIndex -> proposerConfigFuture .thenApply( proposerConfig -> - buildBeaconPreparableProposerList( - proposerConfig, blsPublicKeyIntegerMap)) + buildBeaconPreparableProposerList(proposerConfig, publicKeyToIndex)) .exceptionally( throwable -> { LOG.warn( "An error occurred while obtaining proposer config", throwable); return buildBeaconPreparableProposerList( - Optional.empty(), blsPublicKeyIntegerMap); - }) - .join()) + Optional.empty(), publicKeyToIndex); + })) .thenAccept(validatorApiChannel::prepareBeaconProposer) .finish(VALIDATOR_LOGGER::beaconProposerPreparationFailed); } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java index fff3663aca7..0a92374b516 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java @@ -15,7 +15,6 @@ import java.util.Optional; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import tech.pegasys.teku.infrastructure.async.AsyncRunner; @@ -30,7 +29,7 @@ public abstract class AbstractProposerConfigProvider implements ProposerConfigPr private final AsyncRunner asyncRunner; protected final ProposerConfigLoader proposerConfigLoader; private Optional lastProposerConfig = Optional.empty(); - private final AtomicBoolean requestInProgress = new AtomicBoolean(false); + private Optional>> futureProposerConfig = Optional.empty(); AbstractProposerConfigProvider( final AsyncRunner asyncRunner, @@ -42,42 +41,47 @@ public abstract class AbstractProposerConfigProvider implements ProposerConfigPr } @Override - public SafeFuture> getProposerConfig() { + public synchronized SafeFuture> getProposerConfig() { if (lastProposerConfig.isPresent() && !refresh) { return SafeFuture.completedFuture(lastProposerConfig); } - if (!requestInProgress.compareAndSet(false, true)) { + if (futureProposerConfig.isPresent()) { if (lastProposerConfig.isPresent()) { LOG.warn("A proposer config load is in progress, providing last loaded config"); return SafeFuture.completedFuture(lastProposerConfig); } - return SafeFuture.failedFuture( - new RuntimeException( - "A proposer config load is in progress and there is no previously loaded config")); + return futureProposerConfig.get(); } - - return asyncRunner - .runAsync( - () -> { - lastProposerConfig = Optional.of(internalGetProposerConfig()); - return lastProposerConfig; - }) - .orTimeout(30, TimeUnit.SECONDS) - .exceptionally( - throwable -> { - if (lastProposerConfig.isPresent()) { - LOG.warn( - "An error occurred while obtaining config, providing last loaded config", - throwable); - return lastProposerConfig; - } - throw new RuntimeException( - "An error occurred while obtaining config and there is no previously loaded config", - throwable); - }) - .thenPeek(__ -> LOG.info("proposer config successfully loaded")) - .alwaysRun(() -> requestInProgress.set(false)); + futureProposerConfig = + Optional.of( + asyncRunner + .runAsync( + () -> { + lastProposerConfig = Optional.of(internalGetProposerConfig()); + return lastProposerConfig; + }) + .orTimeout(30, TimeUnit.SECONDS) + .exceptionally( + throwable -> { + if (lastProposerConfig.isPresent()) { + LOG.warn( + "An error occurred while obtaining config, providing last loaded config", + throwable); + return lastProposerConfig; + } + throw new RuntimeException( + "An error occurred while obtaining config and there is no previously loaded config", + throwable); + }) + .thenPeek(__ -> LOG.info("proposer config successfully loaded")) + .alwaysRun( + () -> { + synchronized (this) { + futureProposerConfig = Optional.empty(); + } + })); + return futureProposerConfig.get(); } protected abstract ProposerConfig internalGetProposerConfig(); diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/FileProposerConfigProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/FileProposerConfigProvider.java deleted file mode 100644 index 4a22fcf7011..00000000000 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/FileProposerConfigProvider.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright 2022 ConsenSys AG. - * - * 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.proposerconfig; - -import java.io.File; -import tech.pegasys.teku.infrastructure.async.AsyncRunner; -import tech.pegasys.teku.validator.client.ProposerConfig; -import tech.pegasys.teku.validator.client.proposerconfig.loader.ProposerConfigLoader; - -public class FileProposerConfigProvider extends AbstractProposerConfigProvider { - private final File source; - - FileProposerConfigProvider( - final AsyncRunner asyncRunner, - final boolean refresh, - final ProposerConfigLoader proposerConfigLoader, - final File source) { - super(asyncRunner, refresh, proposerConfigLoader); - this.source = source; - } - - @Override - protected ProposerConfig internalGetProposerConfig() { - return proposerConfigLoader.getProposerConfig(source); - } -} diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProvider.java index cdc1cdfa715..6c82a5d2534 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProvider.java @@ -32,14 +32,18 @@ static ProposerConfigProvider create( final Optional source) { if (source.isPresent()) { + URL sourceUrl; try { - URL sourceUrl = new URL(source.get()); + sourceUrl = new URL(source.get()); return new UrlProposerConfigProvider(asyncRunner, refresh, proposerConfigLoader, sourceUrl); - } catch (MalformedURLException e) { - File sourceFile = new File(source.get()); - return new FileProposerConfigProvider( - asyncRunner, refresh, proposerConfigLoader, sourceFile); + } catch (MalformedURLException e1) { + try { + sourceUrl = new File(source.get()).toURI().toURL(); + } catch (MalformedURLException e2) { + throw new RuntimeException("Unable to translate file to URL", e2); + } } + return new UrlProposerConfigProvider(asyncRunner, refresh, proposerConfigLoader, sourceUrl); } return NOOP; } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java index b50b460c286..54ca67f4202 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoader.java @@ -14,7 +14,6 @@ package tech.pegasys.teku.validator.client.proposerconfig.loader; import com.fasterxml.jackson.databind.ObjectMapper; -import java.io.File; import java.io.IOException; import java.net.URL; import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException; @@ -33,22 +32,13 @@ public ProposerConfigLoader(final ObjectMapper objectMapper) { this.objectMapper = objectMapper; } - public ProposerConfig getProposerConfig(final File source) { - try { - return objectMapper.readValue(source, ProposerConfig.class); - } catch (IOException ex) { - throw new InvalidConfigurationException( - "Failed to load proposer config from file: " + source, ex); - } - } - public ProposerConfig getProposerConfig(final URL source) { try { return objectMapper.readValue(source, ProposerConfig.class); } catch (IOException ex) { throw new InvalidConfigurationException( - "Failed to load proposer config from URL: " + "Failed to load proposer config from: " + UrlSanitizer.sanitizePotentialUrl(source.toString()), ex); } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProviderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProviderTest.java index dc7db7965d6..41b612c032f 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProviderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProviderTest.java @@ -19,6 +19,8 @@ import com.google.common.collect.ImmutableMap; import java.io.File; +import java.net.MalformedURLException; +import java.net.URL; import java.util.Optional; import org.junit.jupiter.api.Test; import tech.pegasys.teku.infrastructure.async.SafeFuture; @@ -44,7 +46,11 @@ public class ProposerConfigProviderTest { private final ProposerConfigProvider proposerConfigProvider = ProposerConfigProvider.create(asyncRunner, true, proposerConfigLoader, Optional.of(SOURCE)); - private final File sourceFile = new File(SOURCE); + private final URL sourceUrl; + + public ProposerConfigProviderTest() throws MalformedURLException { + sourceUrl = new File(SOURCE).toURI().toURL(); + } @Test void getProposerConfig_shouldReturnConfig() { @@ -53,7 +59,7 @@ void getProposerConfig_shouldReturnConfig() { assertThat(futureMaybeConfig).isNotCompleted(); - when(proposerConfigLoader.getProposerConfig(sourceFile)).thenReturn(proposerConfigA); + when(proposerConfigLoader.getProposerConfig(sourceUrl)).thenReturn(proposerConfigA); asyncRunner.executeQueuedActions(); assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); @@ -64,7 +70,7 @@ void getProposerConfig_onErrorShouldThrowWhenNoLastConfigAvailable() { SafeFuture> futureMaybeConfig = proposerConfigProvider.getProposerConfig(); - when(proposerConfigLoader.getProposerConfig(sourceFile)) + when(proposerConfigLoader.getProposerConfig(sourceUrl)) .thenThrow(new RuntimeException("error")); asyncRunner.executeQueuedActions(); @@ -76,14 +82,14 @@ void getProposerConfig_onErrorShouldReturnLastConfigWhenLastConfigAvailable() { SafeFuture> futureMaybeConfig = proposerConfigProvider.getProposerConfig(); - when(proposerConfigLoader.getProposerConfig(sourceFile)).thenReturn(proposerConfigA); + when(proposerConfigLoader.getProposerConfig(sourceUrl)).thenReturn(proposerConfigA); asyncRunner.executeQueuedActions(); assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); futureMaybeConfig = proposerConfigProvider.getProposerConfig(); - when(proposerConfigLoader.getProposerConfig(sourceFile)) + when(proposerConfigLoader.getProposerConfig(sourceUrl)) .thenThrow(new RuntimeException("error")); asyncRunner.executeQueuedActions(); @@ -91,15 +97,16 @@ void getProposerConfig_onErrorShouldReturnLastConfigWhenLastConfigAvailable() { } @Test - void getProposerConfig_onConcurrentCallsShouldThrowWhenNoLastConfigAvailable() { + void getProposerConfig_onConcurrentCallsShouldMergeFuturesWhenNoLastConfigAvailable() { SafeFuture> futureMaybeConfig = proposerConfigProvider.getProposerConfig(); SafeFuture> futureMaybeConfig2 = proposerConfigProvider.getProposerConfig(); - assertThat(futureMaybeConfig2).isCompletedExceptionally(); + assertThat(futureMaybeConfig2).isEqualTo(futureMaybeConfig); + assertThat(futureMaybeConfig2).isNotCompleted(); - when(proposerConfigLoader.getProposerConfig(sourceFile)).thenReturn(proposerConfigA); + when(proposerConfigLoader.getProposerConfig(sourceUrl)).thenReturn(proposerConfigA); asyncRunner.executeQueuedActions(); assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); @@ -110,7 +117,7 @@ void getProposerConfig_onConcurrentCallsShouldReturnLastConfigWhenLastConfigAvai SafeFuture> futureMaybeConfig = proposerConfigProvider.getProposerConfig(); - when(proposerConfigLoader.getProposerConfig(sourceFile)).thenReturn(proposerConfigA); + when(proposerConfigLoader.getProposerConfig(sourceUrl)).thenReturn(proposerConfigA); asyncRunner.executeQueuedActions(); assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); @@ -121,7 +128,7 @@ void getProposerConfig_onConcurrentCallsShouldReturnLastConfigWhenLastConfigAvai assertThat(futureMaybeConfig2).isCompletedWithValue(Optional.of(proposerConfigA)); - when(proposerConfigLoader.getProposerConfig(sourceFile)).thenReturn(proposerConfigB); + when(proposerConfigLoader.getProposerConfig(sourceUrl)).thenReturn(proposerConfigB); asyncRunner.executeQueuedActions(); assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigB)); @@ -138,12 +145,12 @@ void getProposerConfig_shouldAlwaysReturnFirstValidConfigWhenRefreshIsFalse() { assertThat(futureMaybeConfig).isNotCompleted(); - when(proposerConfigLoader.getProposerConfig(sourceFile)).thenReturn(proposerConfigA); + when(proposerConfigLoader.getProposerConfig(sourceUrl)).thenReturn(proposerConfigA); asyncRunner.executeQueuedActions(); assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); - when(proposerConfigLoader.getProposerConfig(sourceFile)).thenReturn(proposerConfigB); + when(proposerConfigLoader.getProposerConfig(sourceUrl)).thenReturn(proposerConfigB); futureMaybeConfig = proposerConfigProvider.getProposerConfig(); assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoaderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoaderTest.java index 0db33ecefa9..07a241229a0 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoaderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/loader/ProposerConfigLoaderTest.java @@ -17,7 +17,6 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.google.common.io.Resources; -import java.io.File; import java.net.URL; import java.util.Optional; import org.junit.jupiter.api.Test; @@ -35,13 +34,6 @@ void shouldLoadValidConfigFromUrl() { validateContent1(loader.getProposerConfig(resource)); } - @Test - void shouldLoadValidConfigFromFile() { - final URL resource = Resources.getResource("proposerConfigValid1.json"); - - validateContent1(loader.getProposerConfig(new File(resource.getPath()))); - } - @Test void shouldLoadConfigWithEmptyProposerConfig() { final URL resource = Resources.getResource("proposerConfigValid2.json"); From 5f89f58aaa44bfdee6cfab6e57ae87b28cbf96b0 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Mon, 31 Jan 2022 11:24:14 +0100 Subject: [PATCH 12/14] another concurrency simplification --- .../AbstractProposerConfigProvider.java | 6 ++--- .../ProposerConfigProviderTest.java | 24 +------------------ 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java index 0a92374b516..eba32d4dec8 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/proposerconfig/AbstractProposerConfigProvider.java @@ -47,10 +47,8 @@ public synchronized SafeFuture> getProposerConfig() { } if (futureProposerConfig.isPresent()) { - if (lastProposerConfig.isPresent()) { - LOG.warn("A proposer config load is in progress, providing last loaded config"); - return SafeFuture.completedFuture(lastProposerConfig); - } + LOG.warn( + "A proposer config load is already progress, waiting it instead of generating a new request"); return futureProposerConfig.get(); } futureProposerConfig = diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProviderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProviderTest.java index 41b612c032f..76aa9938148 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProviderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/proposerconfig/ProposerConfigProviderTest.java @@ -97,7 +97,7 @@ void getProposerConfig_onErrorShouldReturnLastConfigWhenLastConfigAvailable() { } @Test - void getProposerConfig_onConcurrentCallsShouldMergeFuturesWhenNoLastConfigAvailable() { + void getProposerConfig_onConcurrentCallsShouldMergeFutures() { SafeFuture> futureMaybeConfig = proposerConfigProvider.getProposerConfig(); @@ -112,28 +112,6 @@ void getProposerConfig_onConcurrentCallsShouldMergeFuturesWhenNoLastConfigAvaila assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); } - @Test - void getProposerConfig_onConcurrentCallsShouldReturnLastConfigWhenLastConfigAvailable() { - SafeFuture> futureMaybeConfig = - proposerConfigProvider.getProposerConfig(); - - when(proposerConfigLoader.getProposerConfig(sourceUrl)).thenReturn(proposerConfigA); - asyncRunner.executeQueuedActions(); - - assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigA)); - - futureMaybeConfig = proposerConfigProvider.getProposerConfig(); - SafeFuture> futureMaybeConfig2 = - proposerConfigProvider.getProposerConfig(); - - assertThat(futureMaybeConfig2).isCompletedWithValue(Optional.of(proposerConfigA)); - - when(proposerConfigLoader.getProposerConfig(sourceUrl)).thenReturn(proposerConfigB); - asyncRunner.executeQueuedActions(); - - assertThat(futureMaybeConfig).isCompletedWithValue(Optional.of(proposerConfigB)); - } - @Test void getProposerConfig_shouldAlwaysReturnFirstValidConfigWhenRefreshIsFalse() { final ProposerConfigProvider proposerConfigProvider = From 29db0c0ef8bb5f33bf1dac088e2b74bc8ab312be Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Mon, 31 Jan 2022 21:38:47 +0100 Subject: [PATCH 13/14] rely on internal ownedValidator to calculate validator indices --- .../validator/client/AbstractDutyLoader.java | 2 +- .../client/ValidatorIndexProvider.java | 27 ++++++++++++------- .../client/AbstractDutySchedulerTest.java | 2 +- .../client/AttestationDutyLoaderTest.java | 2 +- .../client/SyncCommitteeDutyLoaderTest.java | 2 +- .../client/ValidatorIndexProviderTest.java | 2 +- 6 files changed, 23 insertions(+), 14 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/AbstractDutyLoader.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/AbstractDutyLoader.java index 15dd5180246..100365bfebf 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/AbstractDutyLoader.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/AbstractDutyLoader.java @@ -38,7 +38,7 @@ protected AbstractDutyLoader( public SafeFuture> loadDutiesForEpoch(final UInt64 epoch) { LOG.trace("Requesting duties for epoch {}", epoch); return validatorIndexProvider - .getValidatorIndices(validators.getPublicKeys()) + .getValidatorIndices() .thenCompose( validatorIndices -> { if (validatorIndices.isEmpty()) { diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorIndexProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorIndexProvider.java index 8f7371e1f69..0b8779f2db2 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorIndexProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorIndexProvider.java @@ -13,7 +13,6 @@ package tech.pegasys.teku.validator.client; -import static java.util.Collections.unmodifiableMap; import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toList; @@ -25,6 +24,8 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Function; +import java.util.stream.Collectors; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import tech.pegasys.teku.bls.BLSPublicKey; @@ -37,7 +38,7 @@ public class ValidatorIndexProvider { private static final Logger LOG = LogManager.getLogger(); public static final Duration RETRY_DELAY = Duration.ofSeconds(5); - private final OwnedValidators validators; + private final OwnedValidators ownedValidators; private final ValidatorApiChannel validatorApiChannel; private final AsyncRunner asyncRunner; private final Map validatorIndexesByPublicKey = new ConcurrentHashMap<>(); @@ -46,10 +47,10 @@ public class ValidatorIndexProvider { private final SafeFuture firstSuccessfulRequest = new SafeFuture<>(); public ValidatorIndexProvider( - final OwnedValidators validators, + final OwnedValidators ownedValidators, final ValidatorApiChannel validatorApiChannel, final AsyncRunner asyncRunner) { - this.validators = validators; + this.ownedValidators = ownedValidators; this.validatorApiChannel = validatorApiChannel; this.asyncRunner = asyncRunner; } @@ -82,7 +83,7 @@ public void lookupValidators() { } private Collection getUnknownValidators() { - return Sets.difference(validators.getPublicKeys(), validatorIndexesByPublicKey.keySet()); + return Sets.difference(ownedValidators.getPublicKeys(), validatorIndexesByPublicKey.keySet()); } private void logNewValidatorIndices(final Map knownValidators) { @@ -100,15 +101,23 @@ public Optional getValidatorIndex(final BLSPublicKey publicKey) { return Optional.ofNullable(validatorIndexesByPublicKey.get(publicKey)); } - public SafeFuture> getValidatorIndices( - final Collection publicKeys) { + public SafeFuture> getValidatorIndices() { // Wait for at least one successful load of validator indices before attempting to read return firstSuccessfulRequest.thenApply( __ -> - publicKeys.stream().flatMap(key -> getValidatorIndex(key).stream()).collect(toList())); + ownedValidators.getActiveValidators().stream() + .flatMap(validator -> getValidatorIndex(validator.getPublicKey()).stream()) + .collect(toList())); } public SafeFuture> getValidatorIndexesByPublicKey() { - return firstSuccessfulRequest.thenApply(__ -> unmodifiableMap(validatorIndexesByPublicKey)); + // Wait for at least one successful load of validator indices before attempting to read + return firstSuccessfulRequest.thenApply( + __ -> + ownedValidators.getActiveValidators().stream() + .map(Validator::getPublicKey) + .collect( + Collectors.toMap( + Function.identity(), validatorIndexesByPublicKey::get))); } } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/AbstractDutySchedulerTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/AbstractDutySchedulerTest.java index 31ebaab0027..c2720bf02f9 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/AbstractDutySchedulerTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/AbstractDutySchedulerTest.java @@ -56,7 +56,7 @@ public abstract class AbstractDutySchedulerTest { @BeforeEach public void setUp() { - when(validatorIndexProvider.getValidatorIndices(VALIDATOR_KEYS)) + when(validatorIndexProvider.getValidatorIndices()) .thenReturn(SafeFuture.completedFuture(VALIDATOR_INDICES)); final SafeFuture rejectAggregationSignature = SafeFuture.failedFuture(new UnsupportedOperationException("This test ignores aggregation")); diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/AttestationDutyLoaderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/AttestationDutyLoaderTest.java index ff46a0a6ff9..4a12b8501b4 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/AttestationDutyLoaderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/AttestationDutyLoaderTest.java @@ -79,7 +79,7 @@ class AttestationDutyLoaderTest { @BeforeEach void setUp() { - when(validatorIndexProvider.getValidatorIndices(any())) + when(validatorIndexProvider.getValidatorIndices()) .thenReturn(SafeFuture.completedFuture(VALIDATOR_INDICES)); when(forkProvider.getForkInfo(any())).thenReturn(SafeFuture.completedFuture(forkInfo)); } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/SyncCommitteeDutyLoaderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/SyncCommitteeDutyLoaderTest.java index 6dc6efc49fb..9e1119fd714 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/SyncCommitteeDutyLoaderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/SyncCommitteeDutyLoaderTest.java @@ -69,7 +69,7 @@ class SyncCommitteeDutyLoaderTest { @BeforeEach void setUp() { - when(validatorIndexProvider.getValidatorIndices(validators.getPublicKeys())) + when(validatorIndexProvider.getValidatorIndices()) .thenReturn(SafeFuture.completedFuture(validatorIndices)); } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorIndexProviderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorIndexProviderTest.java index fd7e83d8f3b..ee53a4363d7 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorIndexProviderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/ValidatorIndexProviderTest.java @@ -161,7 +161,7 @@ void shouldWaitForFirstSuccessfulRequestBeforeLookingUpValidatorIndices() { final SafeFuture> requestResult = new SafeFuture<>(); when(validatorApiChannel.getValidatorIndices(Set.of(key1))).thenReturn(requestResult); - final SafeFuture> result = provider.getValidatorIndices(List.of(key1)); + final SafeFuture> result = provider.getValidatorIndices(); assertThat(result).isNotDone(); provider.lookupValidators(); From 6194e5a45c55f638104e1af56c99efb83f8d4355 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Tue, 1 Feb 2022 08:45:52 +0100 Subject: [PATCH 14/14] rely on internal ownedValidator to calculate validator indices --- .../validator/client/BeaconProposerPreparer.java | 7 ++++--- .../validator/client/ValidatorIndexProvider.java | 6 ++---- .../client/BeaconProposerPreparerTest.java | 14 ++++++++++++-- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java index 0802fe3e329..94b0bfb91de 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java @@ -87,12 +87,13 @@ public void onSlot(UInt64 slot) { private List buildBeaconPreparableProposerList( Optional maybeProposerConfig, - Map blsPublicKeyIntegerMap) { - return blsPublicKeyIntegerMap.entrySet().stream() + Map> blsPublicKeyToIndexMap) { + return blsPublicKeyToIndexMap.entrySet().stream() + .filter(blsPublicKeyOptionalEntry -> blsPublicKeyOptionalEntry.getValue().isPresent()) .map( blsPublicKeyIntegerEntry -> new BeaconPreparableProposer( - UInt64.valueOf(blsPublicKeyIntegerEntry.getValue()), + UInt64.valueOf(blsPublicKeyIntegerEntry.getValue().get()), getFeeRecipient(maybeProposerConfig, blsPublicKeyIntegerEntry.getKey()))) .collect(Collectors.toList()); } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorIndexProvider.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorIndexProvider.java index 0b8779f2db2..0c19990fe4f 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorIndexProvider.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorIndexProvider.java @@ -110,14 +110,12 @@ public SafeFuture> getValidatorIndices() { .collect(toList())); } - public SafeFuture> getValidatorIndexesByPublicKey() { + public SafeFuture>> getValidatorIndexesByPublicKey() { // Wait for at least one successful load of validator indices before attempting to read return firstSuccessfulRequest.thenApply( __ -> ownedValidators.getActiveValidators().stream() .map(Validator::getPublicKey) - .collect( - Collectors.toMap( - Function.identity(), validatorIndexesByPublicKey::get))); + .collect(Collectors.toMap(Function.identity(), this::getValidatorIndex))); } } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/BeaconProposerPreparerTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/BeaconProposerPreparerTest.java index a387722cb9a..b9dad0e0cde 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/BeaconProposerPreparerTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/BeaconProposerPreparerTest.java @@ -66,10 +66,20 @@ void setUp(SpecContext specContext) { specContext.getDataStructureUtil().randomPublicKey(), mock(Signer.class), Optional::empty); + Validator validatorWithoutIndex = + new Validator( + specContext.getDataStructureUtil().randomPublicKey(), + mock(Signer.class), + Optional::empty); - Map validatorIndexesByPublicKey = + Map> validatorIndexesByPublicKey = Map.of( - validator1.getPublicKey(), validator1Index, validator2.getPublicKey(), validator2Index); + validator1.getPublicKey(), + Optional.of(validator1Index), + validator2.getPublicKey(), + Optional.of(validator2Index), + validatorWithoutIndex.getPublicKey(), + Optional.empty()); defaultFeeRecipient = specContext.getDataStructureUtil().randomEth1Address(); defaultFeeRecipientConfig = specContext.getDataStructureUtil().randomEth1Address();