Remove --validator-ids option from watermark-repair subcommand (#909)

This ensures all validators are updated by the subcommand. Preparing for adding high-watermark features to the watermark-repair subcommand.
Consensys · Sep 12, 2023 · a383c32 · a383c32
1 parent 72a2318
commit a383c32
Show file tree

Hide file tree

Showing 5 changed files with 5 additions and 106 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 ## Next Release
 
+### Breaking Changes
+- Remove --validator-ids option from watermark-repair subcommand [#909](https://github.com/Consensys/web3signer/pull/909)
+
 ### Features Added
 - Aws bulk loading for secp256k1 keys in eth1 mode [#889](https://github.com/Consensys/web3signer/pull/889)
 

diff --git a/.../src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsConfigFileImpl.java b/.../src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsConfigFileImpl.java
@@ -50,7 +50,6 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Consumer;
-import java.util.stream.Collectors;
 
 import org.apache.commons.io.FileUtils;
 
@@ -280,9 +279,6 @@ private CommandArgs createSubCommandArgs() {
               YAML_NUMERIC_FMT,
               "eth2.watermark-repair.epoch",
               watermarkRepairParameters.getEpoch()));
-      yamlConfig.append(
-          formatStringList(
-              "eth2.watermark-repair.validator-ids", watermarkRepairParameters.getValidators()));
     }
 
     return new CommandArgs(params, yamlConfig.toString());
@@ -631,14 +627,6 @@ private String awsKmsBulkLoadingOptions(final AwsVaultParameters awsVaultParamet
     return yamlConfig.toString();
   }
 
-  private String formatStringList(final String key, final List<String> stringList) {
-    return stringList.isEmpty()
-        ? String.format("%s: []%n", key)
-        : String.format(
-            "%s: [\"%s\"]%n",
-            key, stringList.stream().map(s -> "\"" + s + "\"").collect(Collectors.joining(",")));
-  }
-
   private static class CommandArgs {
     private final List<String> params;
     private final String yamlConfig;

diff --git a/...sts/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsDefaultImpl.java b/...sts/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsDefaultImpl.java
@@ -446,10 +446,6 @@ private List<String> createSubCommandArgs() {
       params.add(Long.toString(watermarkRepairParameters.getEpoch()));
       params.add("--slot");
       params.add(Long.toString(watermarkRepairParameters.getSlot()));
-      if (!watermarkRepairParameters.getValidators().isEmpty()) {
-        params.add(
-            "--validator-ids" + "=" + String.join(",", watermarkRepairParameters.getValidators()));
-      }
     }
 
     return params;

diff --git a/.../src/test/java/tech/pegasys/web3signer/tests/WatermarkRepairSubCommandAcceptanceTest.java b/.../src/test/java/tech/pegasys/web3signer/tests/WatermarkRepairSubCommandAcceptanceTest.java
@@ -106,54 +106,6 @@ void allLowWatermarksAreUpdated(@TempDir final Path testDirectory) throws URISyn
     assertThat(validator2.get("target_epoch")).isEqualTo(epoch);
   }
 
-  @Test
-  void onlySpecifiedWatermarksAreUpdated(@TempDir final Path testDirectory)
-      throws URISyntaxException {
-    setupSigner(testDirectory);
-
-    importSlashingProtectionData(testDirectory);
-
-    final SignerConfigurationBuilder repairBuilder = new SignerConfigurationBuilder();
-    repairBuilder.withMode("eth2");
-    repairBuilder.withSlashingEnabled(true);
-    repairBuilder.withSlashingProtectionDbUrl(signer.getSlashingDbUrl());
-    repairBuilder.withSlashingProtectionDbUsername("postgres");
-    repairBuilder.withSlashingProtectionDbPassword("postgres");
-    repairBuilder.withWatermarkRepairParameters(
-        new WatermarkRepairParameters(
-            20000,
-            30000,
-            List.of(
-                "0x98d083489b3b06b8740da2dfec5cc3c01b2086363fe023a9d7dc1f907633b1ff11f7b99b19e0533e969862270061d884")));
-    repairBuilder.withHttpPort(12345); // prevent wait for Ports file in AT
-
-    final Signer watermarkRepairSigner = new Signer(repairBuilder.build(), null);
-    watermarkRepairSigner.start();
-    waitFor(() -> assertThat(watermarkRepairSigner.isRunning()).isFalse());
-
-    final Map<Object, List<Map<String, Object>>> watermarks = getWatermarks();
-
-    assertThat(watermarks).hasSize(2);
-
-    final Map<String, Object> validator1 =
-        watermarks
-            .get(
-                "0x8f3f44b74d316c3293cced0c48c72e021ef8d145d136f2908931090e7181c3b777498128a348d07b0b9cd3921b5ca537")
-            .get(0);
-    assertThat(validator1.get("slot")).isEqualTo(BigDecimal.valueOf(12345));
-    assertThat(validator1.get("source_epoch")).isEqualTo(BigDecimal.valueOf(5));
-    assertThat(validator1.get("target_epoch")).isEqualTo(BigDecimal.valueOf(6));
-
-    final Map<String, Object> validator2 =
-        watermarks
-            .get(
-                "0x98d083489b3b06b8740da2dfec5cc3c01b2086363fe023a9d7dc1f907633b1ff11f7b99b19e0533e969862270061d884")
-            .get(0);
-    assertThat(validator2.get("slot")).isEqualTo(BigDecimal.valueOf(20000));
-    assertThat(validator2.get("source_epoch")).isEqualTo(BigDecimal.valueOf(30000));
-    assertThat(validator2.get("target_epoch")).isEqualTo(BigDecimal.valueOf(30000));
-  }
-
   private Map<Object, List<Map<String, Object>>> getWatermarks() {
     final Jdbi jdbi = Jdbi.create(signer.getSlashingDbUrl(), DB_USERNAME, DB_PASSWORD);
     return jdbi.withHandle(

diff --git a/...n/java/tech/pegasys/web3signer/commandline/subcommands/Eth2WatermarkRepairSubCommand.java b/...n/java/tech/pegasys/web3signer/commandline/subcommands/Eth2WatermarkRepairSubCommand.java
@@ -18,23 +18,17 @@
 import tech.pegasys.web3signer.slashingprotection.dao.Validator;
 import tech.pegasys.web3signer.slashingprotection.dao.ValidatorsDao;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.stream.Collectors;
 
-import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.units.bigints.UInt64;
 import org.jdbi.v3.core.Jdbi;
 import picocli.CommandLine;
 import picocli.CommandLine.Command;
 import picocli.CommandLine.HelpCommand;
-import picocli.CommandLine.Model.CommandSpec;
 import picocli.CommandLine.Option;
-import picocli.CommandLine.ParameterException;
-import picocli.CommandLine.Spec;
 
 @Command(
     name = "watermark-repair",
@@ -46,8 +40,6 @@
 public class Eth2WatermarkRepairSubCommand implements Runnable {
   private static final Logger LOG = LogManager.getLogger();
 
-  @Spec private CommandSpec commandSpec;
-
   @CommandLine.ParentCommand private Eth2SubCommand eth2Config;
 
   @Option(
@@ -64,25 +56,10 @@ public class Eth2WatermarkRepairSubCommand implements Runnable {
       arity = "1")
   Long slot;
 
-  @Option(
-      names = "--validator-ids",
-      paramLabel = "<publicKey>",
-      description =
-          "List of validator public keys as hexadecimal to apply low watermark update to. If none are specified"
-              + " then all validators low watermarks will be updated (default: ${DEFAULT-VALUE}).",
-      split = ",",
-      arity = "0..*")
-  List<String> validatorIds = new ArrayList<>();
-
   @Override
   public void run() {
     final LowWatermarkDao lowWatermarkDao = new LowWatermarkDao();
     final ValidatorsDao validatorsDao = new ValidatorsDao();
-    final List<Bytes> validatorIdPublicKeys =
-        validatorIds.stream()
-            .filter(StringUtils::isNotEmpty)
-            .map(this::convertToBytes)
-            .collect(Collectors.toList());
 
     final SlashingProtectionContext slashingProtectionContext =
         SlashingProtectionContextFactory.create(eth2Config.getSlashingProtectionParameters());
@@ -91,14 +68,7 @@ public void run() {
     final List<Validator> allValidators =
         jdbi.inTransaction(h -> validatorsDao.findAllValidators(h).collect(Collectors.toList()));
 
-    final List<Validator> validators =
-        validatorIdPublicKeys.isEmpty()
-            ? allValidators
-            : allValidators.stream()
-                .filter(v -> validatorIdPublicKeys.contains(v.getPublicKey()))
-                .collect(Collectors.toList());
-
-    validators.stream()
+    allValidators.stream()
         .parallel()
         .forEach(
             validator ->
@@ -109,16 +79,6 @@ public void run() {
                       lowWatermarkDao.updateEpochWatermarksFor(
                           h, validator.getId(), UInt64.valueOf(epoch), UInt64.valueOf(epoch));
                     }));
-    LOG.info("Updated low watermark for {} validators", validators.size());
-  }
-
-  private Bytes convertToBytes(final String value) {
-    try {
-      return Bytes.fromHexString(value);
-    } catch (final IllegalArgumentException e) {
-      throw new ParameterException(
-          commandSpec.commandLine(),
-          "Invalid value for validator public key " + value + ". Value must be in hexadecimal.");
-    }
+    LOG.info("Updated low watermark for {} validators", allValidators.size());
   }
 }