From a383c3287d6a1bdf5fb8112c3729574c91a5b792 Mon Sep 17 00:00:00 2001 From: Simon Dudley Date: Tue, 12 Sep 2023 16:06:33 +1000 Subject: [PATCH] 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. --- CHANGELOG.md | 3 ++ .../runner/CmdLineParamsConfigFileImpl.java | 12 ----- .../runner/CmdLineParamsDefaultImpl.java | 4 -- ...termarkRepairSubCommandAcceptanceTest.java | 48 ------------------- .../Eth2WatermarkRepairSubCommand.java | 44 +---------------- 5 files changed, 5 insertions(+), 106 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2c1c6bd4..ef1c202c3 100644 --- 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/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsConfigFileImpl.java b/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsConfigFileImpl.java index 4a70a5a9a..b1b753a10 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsConfigFileImpl.java +++ b/acceptance-tests/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 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 params; private final String yamlConfig; diff --git a/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsDefaultImpl.java b/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsDefaultImpl.java index f4007993c..8444c1cfb 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsDefaultImpl.java +++ b/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsDefaultImpl.java @@ -446,10 +446,6 @@ private List 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/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/WatermarkRepairSubCommandAcceptanceTest.java b/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/WatermarkRepairSubCommandAcceptanceTest.java index 4913c8c77..a2278eda3 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/WatermarkRepairSubCommandAcceptanceTest.java +++ b/acceptance-tests/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>> watermarks = getWatermarks(); - - assertThat(watermarks).hasSize(2); - - final Map 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 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>> getWatermarks() { final Jdbi jdbi = Jdbi.create(signer.getSlashingDbUrl(), DB_USERNAME, DB_PASSWORD); return jdbi.withHandle( diff --git a/commandline/src/main/java/tech/pegasys/web3signer/commandline/subcommands/Eth2WatermarkRepairSubCommand.java b/commandline/src/main/java/tech/pegasys/web3signer/commandline/subcommands/Eth2WatermarkRepairSubCommand.java index b84907240..335fcf7d7 100644 --- a/commandline/src/main/java/tech/pegasys/web3signer/commandline/subcommands/Eth2WatermarkRepairSubCommand.java +++ b/commandline/src/main/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 = "", - 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 validatorIds = new ArrayList<>(); - @Override public void run() { final LowWatermarkDao lowWatermarkDao = new LowWatermarkDao(); final ValidatorsDao validatorsDao = new ValidatorsDao(); - final List 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 allValidators = jdbi.inTransaction(h -> validatorsDao.findAllValidators(h).collect(Collectors.toList())); - final List 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()); } }