diff --git a/CHANGELOG.md b/CHANGELOG.md index cd1c50bae67..a9b4c981a12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,4 +16,6 @@ - Added an info message on startup for the highest supported milestone and associated epoch. - Added jdk 24 docker image build. - Improved performance when scheduling attestations in the beginning of the epoch for a large number of validators. +- Improved configuration loading to use builtin configurations to default any fields we need that were missing from a passed in configuration. + ### Bug Fixes \ No newline at end of file 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 04985a63d65..2e05fc17cfc 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 @@ -14,14 +14,11 @@ package tech.pegasys.teku.networks; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; -import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import java.io.IOException; -import java.io.InputStream; import java.util.function.Consumer; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -33,7 +30,6 @@ import tech.pegasys.teku.spec.config.SpecConfig; import tech.pegasys.teku.spec.config.SpecConfigAndParent; import tech.pegasys.teku.spec.config.SpecConfigLoader; -import tech.pegasys.teku.spec.config.SpecConfigReader; import tech.pegasys.teku.spec.config.builder.SpecConfigBuilder; import tech.pegasys.teku.spec.logic.common.helpers.MiscHelpers; @@ -51,7 +47,6 @@ public class EphemeryNetworkTest { private long expectedMinGenesisTime; private long expectedChainId; private long periodSinceGenesis; - private final SpecConfigReader reader = new SpecConfigReader(); private SpecConfigAndParent configFile; private SpecConfig config; @@ -107,15 +102,6 @@ public void shouldLoadMinGenesisTime() { .isEqualTo(String.valueOf(MIN_GENESIS_TIME)); } - @Test - public void read_missingConfig() { - processFileAsInputStream(getInvalidConfigPath("missingChurnLimit"), this::readConfig); - - assertThatThrownBy(reader::build) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("MIN_PER_EPOCH_CHURN_LIMIT"); - } - @Test void shouldNotUpdateConfigBeforeGenesis() { final Spec spec = @@ -221,40 +207,6 @@ void shouldNotUpdateConfigBeforeNextPeriod() { .isEqualTo(String.valueOf(GENESIS_CHAINID)); } - private static String getInvalidConfigPath(final String name) { - return getConfigPath(name); - } - - private static String getConfigPath(final String name) { - final String path = "tech/pegasys/teku/networks/"; - return path + name + ".yaml"; - } - - private void processFileAsInputStream(final String fileName, final InputStreamHandler handler) { - try (final InputStream inputStream = getFileFromResourceAsStream(fileName)) { - handler.accept(inputStream); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - - private InputStream getFileFromResourceAsStream(final String fileName) { - final InputStream inputStream = getClass().getClassLoader().getResourceAsStream(fileName); - if (inputStream == null) { - throw new IllegalArgumentException("File not found: " + fileName); - } - - return inputStream; - } - - 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/SpecConfigLoader.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigLoader.java index 8ff729fe49c..c80c1e2ea68 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 @@ -33,6 +33,19 @@ public class SpecConfigLoader { private static final Logger LOG = LogManager.getLogger(); private static final List AVAILABLE_PRESETS = List.of("phase0", "altair", "bellatrix", "capella", "deneb", "electra", "fulu"); + private static final List BUILTIN_NETWORKS = + List.of( + "chiado", + "ephemery", + "gnosis", + "holesky", + "hoodi", + "lukso", + "less-swift", + "mainnet", + "minimal", + "sepolia", + "swift"); private static final String CONFIG_PATH = "configs/"; private static final String PRESET_PATH = "presets/"; @@ -50,15 +63,6 @@ public static SpecConfigAndParent loadConfig( 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); - return reader.build(modifier); - } - public static SpecConfigAndParent loadRemoteConfig( final Map config) { final SpecConfigReader reader = new SpecConfigReader(); @@ -84,23 +88,74 @@ public static SpecConfigAndParent loadRemoteConfig( return reader.build(); } - static void processConfig( - final String source, final SpecConfigReader reader, final boolean ignoreUnknownConfigItems) { - try (final InputStream configFile = loadConfigurationFile(source)) { - final Map configValues = reader.readValues(configFile); + static SpecConfigAndParent loadConfig( + final String configName, + final boolean isForgiving, + final Consumer modifier) { + final SpecConfigReader reader = new SpecConfigReader(); + processConfig(configName, reader, isForgiving); + return reader.build(modifier); + } + + // A little extra configuration is able to be loaded from builtin configs. + // isForgiving + // - if config in source has entries we don't need, we will silently ignore them + // - if `CONFIG_NAME` is set in config, and we're missing fields, we will copy them + // Presets will be loaded if specified also, this happens after defaulting from config_name + private static void processConfig( + final String source, final SpecConfigReader reader, final boolean isForgiving) { + try { + final Map configValues = readConfigToMap(source, reader); + + if (!BUILTIN_NETWORKS.contains(source) + && configValues.containsKey(SpecConfigReader.CONFIG_NAME_KEY) + && isForgiving) { + final String builtinConfigName = + (String) configValues.get(SpecConfigReader.CONFIG_NAME_KEY); + if (BUILTIN_NETWORKS.contains(builtinConfigName)) { + final Map builtinConfig = readConfigToMap(builtinConfigName, reader); + boolean firstError = true; + for (final String entry : builtinConfig.keySet()) { + if (!configValues.containsKey(entry)) { + if (firstError) { + firstError = false; + LOG.warn( + "Mapping missing values from {} with our builtin config for {}", + source, + builtinConfigName); + } + LOG.warn("Defaulting {}", entry); + configValues.put(entry, builtinConfig.get(entry)); + } + } + } else { + LOG.debug( + "Skipping defaulting config from CONFIG_NAME {}, as this was not found to be builtin", + builtinConfigName); + } + } final Optional maybePreset = Optional.ofNullable((String) configValues.get(SpecConfigReader.PRESET_KEY)); // 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, isForgiving, preset); } + reader.loadFromMap(configValues, isForgiving); + } catch (IOException e) { + throw new IllegalArgumentException( + "Unable to load configuration for network \"" + source + "\": " + e.getMessage(), e); + } + } - reader.loadFromMap(configValues, ignoreUnknownConfigItems); + private static Map readConfigToMap( + final String source, final SpecConfigReader reader) { + try (final InputStream configFile = loadConfigurationFile(source)) { + return reader.readValues(configFile); } catch (IOException | IllegalArgumentException e) { throw new IllegalArgumentException( - "Unable to load configuration for network \"" + source + "\": " + e.getMessage(), e); + "Unable to load configuration source \"" + source + "\": " + e.getMessage(), e); } } 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 86344250735..28676f8c17f 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 @@ -111,38 +111,13 @@ public class SpecConfigReader { .put(Eth1Address.class, fromString(Eth1Address::fromHexString)) .build(); - @SuppressWarnings("unchecked") - private Object blobScheduleFromList(final Object o) { - final List blobSchedule = new ArrayList<>(); - final List schedule = (List) o; - for (Object entry : schedule) { - if (entry instanceof Map) { - final Map data = (Map) entry; - if (!data.containsKey("EPOCH") - || !data.containsKey("MAX_BLOBS_PER_BLOCK") - || data.size() != 2) { - throw new IllegalArgumentException("Map does not look like a blob schedule"); - } - blobSchedule.add( - new BlobSchedule( - UInt64.valueOf(data.get("EPOCH")), - Integer.parseInt(data.get("MAX_BLOBS_PER_BLOCK")))); - - } else { - throw new IllegalArgumentException("Could not parse entry blob schedule"); - } - } - return blobSchedule; - } - final SpecConfigBuilder configBuilder = SpecConfig.builder(); - public SpecConfigAndParent build() { + SpecConfigAndParent build() { return configBuilder.build(); } - public SpecConfigAndParent build( - final Consumer modifier) { + SpecConfigAndParent build(final Consumer modifier) { modifier.accept(configBuilder); return build(); } @@ -154,14 +129,13 @@ 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) + void readAndApply(final InputStream source, final boolean ignoreUnknownConfigItems) throws IOException { final Map rawValues = readValues(source); loadFromMap(rawValues, ignoreUnknownConfigItems); } - public void loadFromMap( - final Map rawValues, final boolean ignoreUnknownConfigItems) { + void loadFromMap(final Map rawValues, final boolean ignoreUnknownConfigItems) { final Map unprocessedConfig = new HashMap<>(rawValues); final Map apiSpecConfig = new HashMap<>(rawValues); // Remove any keys that we're ignoring @@ -265,7 +239,7 @@ public void loadFromMap( } } - public Map readValues(final InputStream source) throws IOException { + Map readValues(final InputStream source) throws IOException { final YamlConfigReader reader = new YamlConfigReader(); try { return reader.readValues(source); @@ -276,6 +250,30 @@ public Map readValues(final InputStream source) throws IOExcepti } } + @SuppressWarnings("unchecked") + private Object blobScheduleFromList(final Object o) { + final List blobSchedule = new ArrayList<>(); + final List schedule = (List) o; + for (Object entry : schedule) { + if (entry instanceof Map) { + final Map data = (Map) entry; + if (!data.containsKey("EPOCH") + || !data.containsKey("MAX_BLOBS_PER_BLOCK") + || data.size() != 2) { + throw new IllegalArgumentException("Map does not look like a blob schedule"); + } + blobSchedule.add( + new BlobSchedule( + UInt64.valueOf(data.get("EPOCH")), + Integer.parseInt(data.get("MAX_BLOBS_PER_BLOCK")))); + + } else { + throw new IllegalArgumentException("Could not parse entry blob schedule"); + } + } + return blobSchedule; + } + private Stream streamConfigSetters(final Class builderClass) { // Ignore any setters that aren't for individual config entries final Set ignoredSetters = Set.of("rawConfig"); 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 219bf0aef05..5baf03973a8 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 @@ -65,6 +65,59 @@ public void shouldLoadMainnet() throws Exception { assertAllBellatrixFieldsSet(config); } + @Test + public void shouldReadConfigByDefaults(@TempDir final Path tempDir) throws Exception { + final Path configFile = tempDir.resolve("config.yaml"); + Files.writeString( + configFile, + """ + CONFIG_NAME: 'mainnet' + PRESET_BASE: 'mainnet' + """); + final SpecConfig config = + SpecConfigLoader.loadConfig(configFile.toString(), true, __ -> {}).specConfig(); + assertAllBellatrixFieldsSet(config); + assertAllBellatrixFieldsSet(config); + } + + @Test + public void shouldReadConfigByDefaultsWithoutPreset(@TempDir final Path tempDir) + throws Exception { + final Path configFile = tempDir.resolve("config.yaml"); + Files.writeString(configFile, "CONFIG_NAME: 'mainnet'\n"); + final SpecConfig config = + SpecConfigLoader.loadConfig(configFile.toString(), true, __ -> {}).specConfig(); + assertAllBellatrixFieldsSet(config); + assertAllBellatrixFieldsSet(config); + } + + @Test + public void shouldFailToReadConfigWithoutDefaulting(@TempDir final Path tempDir) + throws Exception { + final Path configFile = tempDir.resolve("config.yaml"); + Files.writeString(configFile, "PRESET_BASE: 'mainnet'\n"); + assertThatThrownBy(() -> SpecConfigLoader.loadConfig(configFile.toString(), true, __ -> {})) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining( + "The specified network configuration had missing or invalid values for constants"); + } + + @Test + public void shouldFailToReadConfigWithMissingConfigName(@TempDir final Path tempDir) + throws Exception { + final Path configFile = tempDir.resolve("config.yaml"); + Files.writeString( + configFile, + """ + CONFIG_NAME: 'missing' + PRESET_BASE: 'mainnet' + """); + assertThatThrownBy(() -> SpecConfigLoader.loadConfig(configFile.toString(), true, __ -> {})) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining( + "The specified network configuration had missing or invalid values for constants"); + } + @Test public void shouldLoadMainnetFromFileUrl() throws Exception { final URL url = getMainnetConfigResourceAsUrl();