Added config defaulting to the Config loader

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

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<? extends SpecConfig> 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<SpecConfigBuilder> consumer) {
     final SpecConfigAndParent<? extends SpecConfig> config =
         SpecConfigLoader.loadConfig("ephemery", consumer);

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<String> AVAILABLE_PRESETS =
       List.of("phase0", "altair", "bellatrix", "capella", "deneb", "electra", "fulu");
+  private static final List<String> 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<? extends SpecConfig> loadConfig(
     return loadConfig(configName, true, modifier);
   }
 
-  public static SpecConfigAndParent<? extends SpecConfig> loadConfig(
-      final String configName,
-      final boolean ignoreUnknownConfigItems,
-      final Consumer<SpecConfigBuilder> modifier) {
-    final SpecConfigReader reader = new SpecConfigReader();
-    processConfig(configName, reader, ignoreUnknownConfigItems);
-    return reader.build(modifier);
-  }
-
   public static SpecConfigAndParent<? extends SpecConfig> loadRemoteConfig(
       final Map<String, Object> config) {
     final SpecConfigReader reader = new SpecConfigReader();
@@ -84,23 +88,74 @@ public static SpecConfigAndParent<? extends SpecConfig> loadRemoteConfig(
     return reader.build();
   }
 
-  static void processConfig(
-      final String source, final SpecConfigReader reader, final boolean ignoreUnknownConfigItems) {
-    try (final InputStream configFile = loadConfigurationFile(source)) {
-      final Map<String, Object> configValues = reader.readValues(configFile);
+  static SpecConfigAndParent<? extends SpecConfig> loadConfig(
+      final String configName,
+      final boolean isForgiving,
+      final Consumer<SpecConfigBuilder> 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<String, Object> 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<String, Object> 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<String> 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<String, Object> 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);
     }
   }
 

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> blobSchedule = new ArrayList<>();
-    final List<?> schedule = (List<?>) o;
-    for (Object entry : schedule) {
-      if (entry instanceof Map) {
-        final Map<String, String> data = (Map<String, String>) 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<? extends SpecConfig> build() {
+  SpecConfigAndParent<? extends SpecConfig> build() {
     return configBuilder.build();
   }
 
-  public SpecConfigAndParent<? extends SpecConfig> build(
-      final Consumer<SpecConfigBuilder> modifier) {
+  SpecConfigAndParent<? extends SpecConfig> build(final Consumer<SpecConfigBuilder> modifier) {
     modifier.accept(configBuilder);
     return build();
   }
@@ -154,14 +129,13 @@ public SpecConfigAndParent<? extends SpecConfig> 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<String, Object> rawValues = readValues(source);
     loadFromMap(rawValues, ignoreUnknownConfigItems);
   }
 
-  public void loadFromMap(
-      final Map<String, Object> rawValues, final boolean ignoreUnknownConfigItems) {
+  void loadFromMap(final Map<String, Object> rawValues, final boolean ignoreUnknownConfigItems) {
     final Map<String, Object> unprocessedConfig = new HashMap<>(rawValues);
     final Map<String, Object> apiSpecConfig = new HashMap<>(rawValues);
     // Remove any keys that we're ignoring
@@ -265,7 +239,7 @@ public void loadFromMap(
     }
   }
 
-  public Map<String, Object> readValues(final InputStream source) throws IOException {
+  Map<String, Object> readValues(final InputStream source) throws IOException {
     final YamlConfigReader reader = new YamlConfigReader();
     try {
       return reader.readValues(source);
@@ -276,6 +250,30 @@ public Map<String, Object> readValues(final InputStream source) throws IOExcepti
     }
   }
 
+  @SuppressWarnings("unchecked")
+  private Object blobScheduleFromList(final Object o) {
+    final List<BlobSchedule> blobSchedule = new ArrayList<>();
+    final List<?> schedule = (List<?>) o;
+    for (Object entry : schedule) {
+      if (entry instanceof Map) {
+        final Map<String, String> data = (Map<String, String>) 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<Method> streamConfigSetters(final Class<?> builderClass) {
     // Ignore any setters that aren't for individual config entries
     final Set<String> ignoredSetters = Set.of("rawConfig");

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();