From e225be1368974718327f89083fe883d2145af403 Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Thu, 12 Dec 2024 16:22:25 +0000 Subject: [PATCH 1/2] Remove `loadConfigStrict` --- .../teku/networks/EphemeryNetworkTest.java | 6 +--- .../spec/config/DelegatingSpecConfig.java | 10 ------ .../spec/config/NetworkingSpecConfig.java | 6 ---- .../teku/spec/config/SpecConfigLoader.java | 35 +++++-------------- .../teku/spec/config/SpecConfigPhase0.java | 20 ----------- .../teku/spec/config/SpecConfigReader.java | 14 +++----- .../config/builder/SpecConfigBuilder.java | 16 --------- .../spec/config/SpecConfigLoaderTest.java | 5 ++- .../spec/config/SpecConfigReaderTest.java | 16 ++------- .../schemas/SchemaDefinitionCacheTest.java | 2 +- 10 files changed, 20 insertions(+), 110 deletions(-) diff --git a/ethereum/networks/src/test/java/tech/pegasys/teku/networks/EphemeryNetworkTest.java b/ethereum/networks/src/test/java/tech/pegasys/teku/networks/EphemeryNetworkTest.java index 4cf1a8d9bea..14f9b45e843 100644 --- a/ethereum/networks/src/test/java/tech/pegasys/teku/networks/EphemeryNetworkTest.java +++ b/ethereum/networks/src/test/java/tech/pegasys/teku/networks/EphemeryNetworkTest.java @@ -109,7 +109,7 @@ public void shouldLoadMinGenesisTime() { @Test public void read_missingConfig() { - processFileAsInputStream(getInvalidConfigPath("missingChurnLimit"), this::readConfig); + processFileAsInputStream(getInvalidConfigPath("missingChurnLimit"), reader::readAndApply); assertThatThrownBy(reader::build) .isInstanceOf(IllegalArgumentException.class) @@ -251,10 +251,6 @@ private interface InputStreamHandler { void accept(InputStream inputStream) throws IOException; } - private void readConfig(final InputStream preset) throws IOException { - reader.readAndApply(preset, false); - } - private Spec getSpec(final Consumer consumer) { final SpecConfigAndParent config = SpecConfigLoader.loadConfig("ephemery", consumer); diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/DelegatingSpecConfig.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/DelegatingSpecConfig.java index b513c9b16bf..403dae93aba 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/DelegatingSpecConfig.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/DelegatingSpecConfig.java @@ -327,16 +327,6 @@ public int getMaxChunkSize() { return specConfig.getMaxChunkSize(); } - @Override - public int getTtfbTimeout() { - return specConfig.getTtfbTimeout(); - } - - @Override - public int getRespTimeout() { - return specConfig.getRespTimeout(); - } - @Override public int getAttestationPropagationSlotRange() { return specConfig.getAttestationPropagationSlotRange(); diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/NetworkingSpecConfig.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/NetworkingSpecConfig.java index ef5b229dfcc..a41f0d5f920 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/NetworkingSpecConfig.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/NetworkingSpecConfig.java @@ -27,12 +27,6 @@ public interface NetworkingSpecConfig { int getMinEpochsForBlockRequests(); - // in seconds - int getTtfbTimeout(); - - // in seconds - int getRespTimeout(); - int getAttestationPropagationSlotRange(); // in millis diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigLoader.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigLoader.java index b7aebebb946..f17e4794ae7 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigLoader.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigLoader.java @@ -36,26 +36,14 @@ public class SpecConfigLoader { private static final String CONFIG_PATH = "configs/"; private static final String PRESET_PATH = "presets/"; - public static SpecConfigAndParent loadConfigStrict( - final String configName) { - return loadConfig(configName, false, __ -> {}); - } - public static SpecConfigAndParent loadConfig(final String configName) { return loadConfig(configName, __ -> {}); } public static SpecConfigAndParent loadConfig( final String configName, final Consumer modifier) { - return loadConfig(configName, true, modifier); - } - - public static SpecConfigAndParent loadConfig( - final String configName, - final boolean ignoreUnknownConfigItems, - final Consumer modifier) { final SpecConfigReader reader = new SpecConfigReader(); - processConfig(configName, reader, ignoreUnknownConfigItems); + processConfig(configName, reader); return reader.build(modifier); } @@ -64,7 +52,7 @@ public static SpecConfigAndParent loadRemoteConfig( final SpecConfigReader reader = new SpecConfigReader(); if (config.containsKey(SpecConfigReader.PRESET_KEY)) { try { - applyPreset("remote", reader, true, config.get(SpecConfigReader.PRESET_KEY)); + applyPreset("remote", reader, config.get(SpecConfigReader.PRESET_KEY)); } catch (IOException e) { throw new UncheckedIOException(e); } @@ -72,7 +60,7 @@ public static SpecConfigAndParent loadRemoteConfig( if (config.containsKey(SpecConfigReader.CONFIG_NAME_KEY)) { final String configNameKey = config.get(SpecConfigReader.CONFIG_NAME_KEY); try { - processConfig(configNameKey, reader, true); + processConfig(configNameKey, reader); } catch (IllegalArgumentException exception) { LOG.debug( "Failed to load base configuration from {}, {}", @@ -80,12 +68,11 @@ public static SpecConfigAndParent loadRemoteConfig( exception::getMessage); } } - reader.loadFromMap(config, true); + reader.loadFromMap(config); return reader.build(); } - static void processConfig( - final String source, final SpecConfigReader reader, final boolean ignoreUnknownConfigItems) { + static void processConfig(final String source, final SpecConfigReader reader) { try (final InputStream configFile = loadConfigurationFile(source)) { final Map configValues = reader.readValues(configFile); final Optional maybePreset = @@ -94,10 +81,10 @@ static void processConfig( // Legacy config files won't have a preset field if (maybePreset.isPresent()) { final String preset = maybePreset.get(); - applyPreset(source, reader, ignoreUnknownConfigItems, preset); + applyPreset(source, reader, preset); } - reader.loadFromMap(configValues, ignoreUnknownConfigItems); + reader.loadFromMap(configValues); } catch (IOException | IllegalArgumentException e) { throw new IllegalArgumentException( "Unable to load configuration for network \"" + source + "\": " + e.getMessage(), e); @@ -105,15 +92,11 @@ static void processConfig( } private static void applyPreset( - final String source, - final SpecConfigReader reader, - final boolean ignoreUnknownConfigItems, - final String preset) - throws IOException { + final String source, final SpecConfigReader reader, final String preset) throws IOException { for (String resource : AVAILABLE_PRESETS) { try (final InputStream inputStream = loadPreset(preset, resource).orElse(null)) { if (inputStream != null) { - reader.readAndApply(inputStream, ignoreUnknownConfigItems); + reader.readAndApply(inputStream); } else if (resource.equals("phase0")) { throw new FileNotFoundException( String.format( diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigPhase0.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigPhase0.java index 6d41969704c..52654dfdd29 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigPhase0.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigPhase0.java @@ -107,8 +107,6 @@ public class SpecConfigPhase0 implements SpecConfig { private final int maxRequestBlocks; private final int epochsPerSubnetSubscription; private final int minEpochsForBlockRequests; - private final int ttfbTimeout; - private final int respTimeout; private final int attestationPropagationSlotRange; private final int maximumGossipClockDisparity; private final Bytes4 messageDomainInvalidSnappy; @@ -179,8 +177,6 @@ public SpecConfigPhase0( final int maxRequestBlocks, final int epochsPerSubnetSubscription, final int minEpochsForBlockRequests, - final int ttfbTimeout, - final int respTimeout, final int attestationPropagationSlotRange, final int maximumGossipClockDisparity, final Bytes4 messageDomainInvalidSnappy, @@ -249,8 +245,6 @@ public SpecConfigPhase0( this.maxRequestBlocks = maxRequestBlocks; this.epochsPerSubnetSubscription = epochsPerSubnetSubscription; this.minEpochsForBlockRequests = minEpochsForBlockRequests; - this.ttfbTimeout = ttfbTimeout; - this.respTimeout = respTimeout; this.attestationPropagationSlotRange = attestationPropagationSlotRange; this.maximumGossipClockDisparity = maximumGossipClockDisparity; this.messageDomainInvalidSnappy = messageDomainInvalidSnappy; @@ -585,16 +579,6 @@ public int getMinEpochsForBlockRequests() { return minEpochsForBlockRequests; } - @Override - public int getTtfbTimeout() { - return ttfbTimeout; - } - - @Override - public int getRespTimeout() { - return respTimeout; - } - @Override public int getAttestationPropagationSlotRange() { return attestationPropagationSlotRange; @@ -691,8 +675,6 @@ public boolean equals(final Object o) { && attestationSubnetCount == that.attestationSubnetCount && attestationSubnetExtraBits == that.attestationSubnetExtraBits && attestationSubnetPrefixBits == that.attestationSubnetPrefixBits - && ttfbTimeout == that.ttfbTimeout - && respTimeout == that.respTimeout && attestationPropagationSlotRange == that.attestationPropagationSlotRange && maximumGossipClockDisparity == that.maximumGossipClockDisparity && Objects.equals(eth1FollowDistance, that.eth1FollowDistance) @@ -772,8 +754,6 @@ public int hashCode() { maxChunkSize, maxRequestBlocks, epochsPerSubnetSubscription, - ttfbTimeout, - respTimeout, attestationPropagationSlotRange, maximumGossipClockDisparity, messageDomainInvalidSnappy, diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigReader.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigReader.java index f48941998bd..7177cc78af8 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigReader.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigReader.java @@ -127,14 +127,12 @@ public SpecConfigAndParent build( * @param source The source to read * @throws IOException Thrown if an error occurs reading the source */ - public void readAndApply(final InputStream source, final boolean ignoreUnknownConfigItems) - throws IOException { + public void readAndApply(final InputStream source) throws IOException { final Map rawValues = readValues(source); - loadFromMap(rawValues, ignoreUnknownConfigItems); + loadFromMap(rawValues); } - public void loadFromMap( - final Map rawValues, final boolean ignoreUnknownConfigItems) { + public void loadFromMap(final Map rawValues) { final Map unprocessedConfig = new HashMap<>(rawValues); final Map apiSpecConfig = new HashMap<>(rawValues); // Remove any keys that we're ignoring @@ -220,11 +218,7 @@ public void loadFromMap( if (unprocessedConfig.size() > 0) { final String unknownKeys = String.join(",", unprocessedConfig.keySet()); - if (!ignoreUnknownConfigItems) { - throw new IllegalArgumentException("Detected unknown spec config entries: " + unknownKeys); - } else { - LOG.warn("Ignoring unknown items in network configuration: {}", unknownKeys); - } + LOG.warn("Ignoring unknown items in network configuration: {}", unknownKeys); } } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/builder/SpecConfigBuilder.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/builder/SpecConfigBuilder.java index 5e48e29614c..e731ec925fe 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/builder/SpecConfigBuilder.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/builder/SpecConfigBuilder.java @@ -112,8 +112,6 @@ public class SpecConfigBuilder { private Integer maxChunkSize; private Integer maxRequestBlocks; private Integer epochsPerSubnetSubscription; - private Integer ttfbTimeout; - private Integer respTimeout; private Integer attestationPropagationSlotRange; private Integer maximumGossipClockDisparity; private Bytes4 messageDomainInvalidSnappy; @@ -205,8 +203,6 @@ public SpecConfigAndParent build() { maxRequestBlocks, epochsPerSubnetSubscription, minEpochsForBlockRequests, - ttfbTimeout, - respTimeout, attestationPropagationSlotRange, maximumGossipClockDisparity, messageDomainInvalidSnappy, @@ -279,8 +275,6 @@ private Map getValidationMap() { constants.put("maxRequestBlocks", maxRequestBlocks); constants.put("epochsPerSubnetSubscription", epochsPerSubnetSubscription); constants.put("minEpochsForBlockRequests", minEpochsForBlockRequests); - constants.put("ttfbTimeout", ttfbTimeout); - constants.put("respTimeout", respTimeout); constants.put("attestationPropagationSlotRange", attestationPropagationSlotRange); constants.put("maximumGossipClockDisparity", maximumGossipClockDisparity); constants.put("messageDomainInvalidSnappy", messageDomainInvalidSnappy); @@ -651,16 +645,6 @@ public SpecConfigBuilder minEpochsForBlockRequests(final Integer minEpochsForBlo return this; } - public SpecConfigBuilder ttfbTimeout(final Integer ttfbTimeout) { - this.ttfbTimeout = ttfbTimeout; - return this; - } - - public SpecConfigBuilder respTimeout(final Integer respTimeout) { - this.respTimeout = respTimeout; - return this; - } - public SpecConfigBuilder attestationPropagationSlotRange( final Integer attestationPropagationSlotRange) { this.attestationPropagationSlotRange = attestationPropagationSlotRange; diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/config/SpecConfigLoaderTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/config/SpecConfigLoaderTest.java index d790ef2ecf9..8f9f3c5e8b4 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/config/SpecConfigLoaderTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/config/SpecConfigLoaderTest.java @@ -41,7 +41,7 @@ public class SpecConfigLoaderTest { @ParameterizedTest(name = "{0}") @EnumSource(Eth2Network.class) public void shouldLoadAllKnownNetworks(final Eth2Network network) throws Exception { - final SpecConfig config = SpecConfigLoader.loadConfigStrict(network.configName()).specConfig(); + final SpecConfig config = SpecConfigLoader.loadConfig(network.configName()).specConfig(); // testing latest SpecConfig ensures all fields will be asserted on assertAllFieldsSet(config, SpecConfigElectra.class); } @@ -136,8 +136,7 @@ public void shouldHandleInvalidPresetValue_unknownPreset(@TempDir final Path tem @Test public void shouldBeAbleToOverridePresetValues() { final URL configUrl = SpecConfigLoaderTest.class.getResource("standard/with-overrides.yaml"); - final SpecConfig config = - SpecConfigLoader.loadConfig(configUrl.toString(), false, __ -> {}).specConfig(); + final SpecConfig config = SpecConfigLoader.loadConfig(configUrl.toString()).specConfig(); assertThat(config).isNotNull(); assertThat(config.getMaxCommitteesPerSlot()).isEqualTo(12); // Mainnet preset is 64. } diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/config/SpecConfigReaderTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/config/SpecConfigReaderTest.java index 592102ab47c..d252a84b2da 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/config/SpecConfigReaderTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/config/SpecConfigReaderTest.java @@ -54,20 +54,10 @@ public void read_missingAltairConstant() { } @Test - void read_unknownConstant() { - assertThatThrownBy( - () -> processFileAsInputStream(getInvalidConfigPath("unknownField"), this::readConfig)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Detected unknown spec config entries: UNKNOWN_CONSTANT"); - } - - @Test - void read_ignoringUnknownConstant() { + void read_ignoresUnknownConstant() { Assertions.assertThatCode( () -> { - processFileAsInputStream( - getInvalidConfigPath("unknownField"), - source -> reader.readAndApply(source, true)); + processFileAsInputStream(getInvalidConfigPath("unknownField"), this::readConfig); assertAllAltairFieldsSet(reader.build().specConfig()); }) .doesNotThrowAnyException(); @@ -210,7 +200,7 @@ public void read_invalidBytes4_tooSmall() { } private void readConfig(final InputStream preset) throws IOException { - reader.readAndApply(preset, false); + reader.readAndApply(preset); } private static String getInvalidConfigPath(final String name) { diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/SchemaDefinitionCacheTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/SchemaDefinitionCacheTest.java index adf512454e3..764f66d2224 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/SchemaDefinitionCacheTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/SchemaDefinitionCacheTest.java @@ -53,7 +53,7 @@ public class SchemaDefinitionCacheTest { void shouldGetSchemasForAllMilestonesOnAllNetworks( final Eth2Network network, final SpecMilestone specMilestone) { final SpecConfigAndParent specConfig = - SpecConfigLoader.loadConfigStrict(network.configName()); + SpecConfigLoader.loadConfig(network.configName()); final Spec spec = SpecFactory.create(specConfig); final SchemaDefinitionCache cache = new SchemaDefinitionCache(spec); assertThat(cache.getSchemaDefinition(specMilestone)).isNotNull(); From 61b5232f32cd4657090ac4ca9b144826e5f10b40 Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Thu, 12 Dec 2024 16:49:23 +0000 Subject: [PATCH 2/2] fix assemble --- .../tech/pegasys/teku/cli/subcommand/RemoteSpecLoaderTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/teku/src/test/java/tech/pegasys/teku/cli/subcommand/RemoteSpecLoaderTest.java b/teku/src/test/java/tech/pegasys/teku/cli/subcommand/RemoteSpecLoaderTest.java index a6818600c2c..5747896cae1 100644 --- a/teku/src/test/java/tech/pegasys/teku/cli/subcommand/RemoteSpecLoaderTest.java +++ b/teku/src/test/java/tech/pegasys/teku/cli/subcommand/RemoteSpecLoaderTest.java @@ -78,8 +78,6 @@ void shouldDefaultNetworkConfigThatMovedFromConstants() throws IOException { assertThat(specConfig.getMaxRequestBlocks()).isEqualTo(1024); assertThat(specConfig.getEpochsPerSubnetSubscription()).isEqualTo(256); assertThat(specConfig.getMinEpochsForBlockRequests()).isEqualTo(33024); - assertThat(specConfig.getTtfbTimeout()).isEqualTo(5); - assertThat(specConfig.getRespTimeout()).isEqualTo(10); assertThat(specConfig.getAttestationPropagationSlotRange()).isEqualTo(32); assertThat(specConfig.getMaximumGossipClockDisparity()).isEqualTo(500); }