Updated handling of config values reading bad configuration

CHANGELOG.md

@@ -23,3 +23,4 @@ the [releases page](https://github.com/Consensys/teku/releases).
  been updated to support older GLIBC versions (ie Ubuntu 20.04 and Debian 11).
 
 ### Bug Fixes
+- During network configuration load, all missing fields will now be reported, rather than just the first missing field causing failure.

ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigLoader.java

@@ -30,13 +30,12 @@
 import tech.pegasys.teku.spec.networks.Eth2Presets;
 
 public class SpecConfigLoader {
+ private static final Logger LOG = LogManager.getLogger();
  private static final List<String> AVAILABLE_PRESETS =
  List.of("phase0", "altair", "bellatrix", "capella", "deneb");
  private static final String CONFIG_PATH = "configs/";
  private static final String PRESET_PATH = "presets/";
 
- private static final Logger LOG = LogManager.getLogger();
-
  public static SpecConfig loadConfigStrict(final String configName) {
  return loadConfig(configName, false, __ -> {});
  }
@@ -62,6 +61,13 @@ public static SpecConfig loadConfig(
  public static SpecConfig loadRemoteConfig(
  final Map<String, String> config, final Consumer<SpecConfigBuilder> modifier) {
  final SpecConfigReader reader = new SpecConfigReader();
+ if (config.containsKey(SpecConfigReader.PRESET_KEY)) {
+ try {
+ applyPreset("remote", reader, true, config.get(SpecConfigReader.PRESET_KEY));
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ }
  if (config.containsKey(SpecConfigReader.CONFIG_NAME_KEY)) {
  final String configNameKey = config.get(SpecConfigReader.CONFIG_NAME_KEY);
  try {
@@ -73,13 +79,6 @@ public static SpecConfig loadRemoteConfig(
  exception::getMessage);
  }
  }
- if (config.containsKey(SpecConfigReader.PRESET_KEY)) {
- try {
- applyPreset("remote", reader, true, config.get(SpecConfigReader.PRESET_KEY));
- } catch (IOException e) {
- throw new UncheckedIOException(e);
- }
- }
  reader.loadFromMap(config, true);
  return reader.build(modifier);
  }

ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/builder/AltairBuilder.java

@@ -16,6 +16,11 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static tech.pegasys.teku.spec.config.SpecConfig.FAR_FUTURE_EPOCH;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
 import java.util.function.BiConsumer;
 import tech.pegasys.teku.infrastructure.bytes.Bytes4;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
@@ -95,26 +100,42 @@ public void validate() {
  SpecBuilderUtil.fillMissingValuesWithZeros(this);
  }
 
- SpecBuilderUtil.validateConstant(
- "inactivityPenaltyQuotientAltair", inactivityPenaltyQuotientAltair);
- SpecBuilderUtil.validateConstant(
- "minSlashingPenaltyQuotientAltair", minSlashingPenaltyQuotientAltair);
- SpecBuilderUtil.validateConstant(
- "proportionalSlashingMultiplierAltair", proportionalSlashingMultiplierAltair);
- SpecBuilderUtil.validateConstant("syncCommitteeSize", syncCommitteeSize);
- SpecBuilderUtil.validateConstant("inactivityScoreBias", inactivityScoreBias);
- SpecBuilderUtil.validateConstant("inactivityScoreRecoveryRate", inactivityScoreRecoveryRate);
- SpecBuilderUtil.validateConstant("epochsPerSyncCommitteePeriod", epochsPerSyncCommitteePeriod);
- SpecBuilderUtil.validateConstant("altairForkVersion", altairForkVersion);
- SpecBuilderUtil.validateConstant("altairForkEpoch", altairForkEpoch);
- SpecBuilderUtil.validateConstant("minSyncCommitteeParticipants", minSyncCommitteeParticipants);
+ final List<Optional<String>> maybeErrors = new ArrayList<>();
+ final Map<String, Object> constants = getValidationMap();
+
+ constants.forEach((k, v) -> maybeErrors.add(SpecBuilderUtil.validateConstant(k, v)));
+
+ final List<String> fieldsFailingValidation =
+ maybeErrors.stream().filter(Optional::isPresent).map(Optional::get).toList();
+
+ if (!fieldsFailingValidation.isEmpty()) {
+ throw new IllegalArgumentException(
+ String.format(
+ "Errors were found validating configuration, missing or invalid values for constants %s",
+ String.join(", ", fieldsFailingValidation)));
+ }
 
  // Config items were added after launch so provide defaults to preserve compatibility
  if (updateTimeout == null) {
  updateTimeout = epochsPerSyncCommitteePeriod * 32;
  }
  }
 
+ private Map<String, Object> getValidationMap() {
+ final Map<String, Object> constants = new HashMap<>();
+ constants.put("inactivityPenaltyQuotientAltair", inactivityPenaltyQuotientAltair);
+ constants.put("minSlashingPenaltyQuotientAltair", minSlashingPenaltyQuotientAltair);
+ constants.put("proportionalSlashingMultiplierAltair", proportionalSlashingMultiplierAltair);
+ constants.put("syncCommitteeSize", syncCommitteeSize);
+ constants.put("inactivityScoreBias", inactivityScoreBias);
+ constants.put("inactivityScoreRecoveryRate", inactivityScoreRecoveryRate);
+ constants.put("epochsPerSyncCommitteePeriod", epochsPerSyncCommitteePeriod);
+ constants.put("altairForkVersion", altairForkVersion);
+ constants.put("altairForkEpoch", altairForkEpoch);
+ constants.put("minSyncCommitteeParticipants", minSyncCommitteeParticipants);
+ return constants;
+ }
+
  @Override
  public void addOverridableItemsToRawConfig(final BiConsumer<String, Object> rawConfig) {
  rawConfig.accept("ALTAIR_FORK_EPOCH", altairForkEpoch);

ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/builder/BellatrixBuilder.java

@@ -18,6 +18,11 @@
 import static tech.pegasys.teku.spec.constants.NetworkConstants.DEFAULT_SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY;
 
 import java.math.BigInteger;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
 import java.util.function.BiConsumer;
 import org.apache.tuweni.bytes.Bytes32;
 import org.apache.tuweni.units.bigints.UInt256;
@@ -108,6 +113,36 @@ public void validate() {
  SpecBuilderUtil.validateConstant("maxTransactionsPerPayload", maxTransactionsPerPayload);
  SpecBuilderUtil.validateConstant("bytesPerLogsBloom", bytesPerLogsBloom);
  SpecBuilderUtil.validateConstant("maxExtraDataBytes", maxExtraDataBytes);
+
+ final List<Optional<String>> maybeErrors = new ArrayList<>();
+ final Map<String, Object> constants = getValidationMap();
+
+ constants.forEach((k, v) -> maybeErrors.add(SpecBuilderUtil.validateConstant(k, v)));
+
+ final List<String> fieldsFailingValidation =
+ maybeErrors.stream().filter(Optional::isPresent).map(Optional::get).toList();
+
+ if (!fieldsFailingValidation.isEmpty()) {
+ throw new IllegalArgumentException(
+ String.format(
+ "The specified network configuration had missing or invalid values for constants %s",
+ String.join(", ", fieldsFailingValidation)));
+ }
+ }
+
+ private Map<String, Object> getValidationMap() {
+ final Map<String, Object> constants = new HashMap<>();
+ constants.put("bellatrixForkVersion", bellatrixForkVersion);
+ constants.put("bellatrixForkEpoch", bellatrixForkEpoch);
+ constants.put("inactivityPenaltyQuotientBellatrix", inactivityPenaltyQuotientBellatrix);
+ constants.put("minSlashingPenaltyQuotientBellatrix", minSlashingPenaltyQuotientBellatrix);
+ constants.put(
+ "proportionalSlashingMultiplierBellatrix", proportionalSlashingMultiplierBellatrix);
+ constants.put("maxBytesPerTransaction", maxBytesPerTransaction);
+ constants.put("maxTransactionsPerPayload", maxTransactionsPerPayload);
+ constants.put("bytesPerLogsBloom", bytesPerLogsBloom);
+ constants.put("maxExtraDataBytes", maxExtraDataBytes);
+ return constants;
  }
 
  @Override

ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/builder/CapellaBuilder.java

@@ -16,6 +16,11 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static tech.pegasys.teku.spec.config.SpecConfig.FAR_FUTURE_EPOCH;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
 import java.util.function.BiConsumer;
 import tech.pegasys.teku.infrastructure.bytes.Bytes4;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
@@ -93,12 +98,31 @@ public void validate() {
  SpecBuilderUtil.fillMissingValuesWithZeros(this);
  }
 
- SpecBuilderUtil.validateConstant("capellaForkVersion", capellaForkVersion);
- SpecBuilderUtil.validateConstant("capellaForkEpoch", capellaForkEpoch);
- SpecBuilderUtil.validateConstant("maxBlsToExecutionChanges", maxBlsToExecutionChanges);
- SpecBuilderUtil.validateConstant("maxWithdrawalsPerPayload", maxWithdrawalsPerPayload);
- SpecBuilderUtil.validateConstant(
- "maxValidatorsPerWithdrawalSweep", maxValidatorsPerWithdrawalSweep);
+ final List<Optional<String>> maybeErrors = new ArrayList<>();
+ final Map<String, Object> constants = getValidationMap();
+
+ constants.forEach((k, v) -> maybeErrors.add(SpecBuilderUtil.validateConstant(k, v)));
+
+ final List<String> fieldsFailingValidation =
+ maybeErrors.stream().filter(Optional::isPresent).map(Optional::get).toList();
+
+ if (!fieldsFailingValidation.isEmpty()) {
+ throw new IllegalArgumentException(
+ String.format(
+ "The specified network configuration had missing or invalid values for constants %s",
+ String.join(", ", fieldsFailingValidation)));
+ }
+ }
+
+ private Map<String, Object> getValidationMap() {
+ final Map<String, Object> constants = new HashMap<>();
+ constants.put("capellaForkVersion", capellaForkVersion);
+ constants.put("capellaForkEpoch", capellaForkEpoch);
+ constants.put("maxBlsToExecutionChanges", maxBlsToExecutionChanges);
+ constants.put("maxWithdrawalsPerPayload", maxWithdrawalsPerPayload);
+ constants.put("maxValidatorsPerWithdrawalSweep", maxValidatorsPerWithdrawalSweep);
+
+ return constants;
  }
 
  @Override

ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/builder/DenebBuilder.java

@@ -16,6 +16,10 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static tech.pegasys.teku.spec.config.SpecConfig.FAR_FUTURE_EPOCH;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.function.BiConsumer;
 import tech.pegasys.teku.infrastructure.bytes.Bytes4;
@@ -147,23 +151,42 @@ public void validate() {
  SpecBuilderUtil.fillMissingValuesWithZeros(this);
  }
 
- SpecBuilderUtil.validateConstant("denebForkEpoch", denebForkEpoch);
- SpecBuilderUtil.validateConstant("denebForkVersion", denebForkVersion);
- SpecBuilderUtil.validateConstant(
- "maxPerEpochActivationChurnLimit", maxPerEpochActivationChurnLimit);
- SpecBuilderUtil.validateConstant("fieldElementsPerBlob", fieldElementsPerBlob);
- SpecBuilderUtil.validateConstant("maxBlobCommitmentsPerBlock", maxBlobCommitmentsPerBlock);
- SpecBuilderUtil.validateConstant("maxBlobsPerBlock", maxBlobsPerBlock);
- SpecBuilderUtil.validateConstant("maxRequestBlocksDeneb", maxRequestBlocksDeneb);
- SpecBuilderUtil.validateConstant("maxRequestBlobSidecars", maxRequestBlobSidecars);
- SpecBuilderUtil.validateConstant(
- "minEpochsForBlobSidecarsRequests", minEpochsForBlobSidecarsRequests);
- SpecBuilderUtil.validateConstant("blobSidecarSubnetCount", blobSidecarSubnetCount);
+ final List<Optional<String>> maybeErrors = new ArrayList<>();
+ final Map<String, Object> constants = getValidationMap();
+
+ constants.forEach((k, v) -> maybeErrors.add(SpecBuilderUtil.validateConstant(k, v)));
+
+ final List<String> fieldsFailingValidation =
+ maybeErrors.stream().filter(Optional::isPresent).map(Optional::get).toList();
+
+ if (!fieldsFailingValidation.isEmpty()) {
+ throw new IllegalArgumentException(
+ String.format(
+ "The specified network configuration had missing or invalid values for constants %s",
+ String.join(", ", fieldsFailingValidation)));
+ }
  if (!denebForkEpoch.equals(SpecConfig.FAR_FUTURE_EPOCH) && !kzgNoop) {
  SpecBuilderUtil.validateRequiredOptional("trustedSetupPath", trustedSetupPath);
  }
  }
 
+ private Map<String, Object> getValidationMap() {
+ final Map<String, Object> constants = new HashMap<>();
+
+ constants.put("denebForkEpoch", denebForkEpoch);
+ constants.put("denebForkVersion", denebForkVersion);
+ constants.put("maxPerEpochActivationChurnLimit", maxPerEpochActivationChurnLimit);
+ constants.put("fieldElementsPerBlob", fieldElementsPerBlob);
+ constants.put("maxBlobCommitmentsPerBlock", maxBlobCommitmentsPerBlock);
+ constants.put("maxBlobsPerBlock", maxBlobsPerBlock);
+ constants.put("maxRequestBlocksDeneb", maxRequestBlocksDeneb);
+ constants.put("maxRequestBlobSidecars", maxRequestBlobSidecars);
+ constants.put("minEpochsForBlobSidecarsRequests", minEpochsForBlobSidecarsRequests);
+ constants.put("blobSidecarSubnetCount", blobSidecarSubnetCount);
+
+ return constants;
+ }
+
  @Override
  public void addOverridableItemsToRawConfig(final BiConsumer<String, Object> rawConfig) {
  rawConfig.accept("DENEB_FORK_EPOCH", denebForkEpoch);

ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/builder/SpecBuilderUtil.java

@@ -13,20 +13,22 @@
 
 package tech.pegasys.teku.spec.config.builder;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static tech.pegasys.teku.spec.config.SpecConfigFormatter.camelToSnakeCase;
 
 import java.lang.reflect.Modifier;
 import java.util.Arrays;
 import java.util.Map;
 import java.util.Optional;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.bytes.Bytes32;
 import org.apache.tuweni.units.bigints.UInt256;
 import tech.pegasys.teku.infrastructure.bytes.Bytes4;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
 
 public class SpecBuilderUtil {
+ private static final Logger LOG = LogManager.getLogger();
  private static final Map<Class<?>, Object> DEFAULT_ZERO_VALUES =
  Map.of(
  UInt64.class,
@@ -45,26 +47,37 @@
  // Placeholder version explicitly doesn't match MainNet (or any other known testnet)
  static final Bytes4 PLACEHOLDER_FORK_VERSION = Bytes4.fromHexString("0x99999999");
 
- static void validateConstant(final String name, final Object value) {
- validateNotNull(name, value);
+ static Optional<String> validateConstant(final String name, final Object value) {
+ return validateNotNull(name, value);
  }
 
- static void validateConstant(final String name, final Long value) {
- validateNotNull(name, value);
- checkArgument(value >= 0, "Long values must be positive");
+ static Optional<String> validateConstant(final String name, final Integer value) {
+ final Optional<String> maybeError = validateNotNull(name, value);
+ if (maybeError.isPresent()) {
+ return maybeError;
+ }
+ if (value < 0) {
+ LOG.error(
+ "Value for constant '{}' ({}) failed to validate - Integer values must be positive",
+ name,
+ value);
+ return Optional.of(name);
+ }
+ return Optional.empty();
  }
 
- static void validateConstant(final String name, final Integer value) {
- validateNotNull(name, value);
- checkArgument(value >= 0, "Integer values must be positive");
+ private static Optional<String> validateNotNull(final String name, final Object value) {
+ if (value == null) {
+ return Optional.of(camelToSnakeCase(name));
+ }
+ return Optional.empty();
  }
 
- static void validateNotNull(final String name, final Object value) {
- checkArgument(value != null, "Missing value for spec constant '%s'", camelToSnakeCase(name));
- }
-
- static <T> void validateRequiredOptional(final String name, final Optional<T> value) {
- checkArgument(value.isPresent(), "Missing value for required '%s'", name);
+ static <T> Optional<String> validateRequiredOptional(final String name, final Optional<T> value) {
+ if (value.isEmpty()) {
+ return Optional.of(name);
+ }
+ return Optional.empty();
  }
 
  static void fillMissingValuesWithZeros(final ForkConfigBuilder<?, ?> builder) {