Revert "Add DepositTreeSnapshot download to checkpoint sync (#7793)" (#…

…7831)
Consensys · Dec 14, 2023 · f3d80b3 · f3d80b3
1 parent 9a38947
commit f3d80b3
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 92 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,5 +15,6 @@ the [releases page](https://github.com/Consensys/teku/releases).
 - Added `--ee-jwt-claim-id` command line option to provide `id` to the execution engine JWT claims
 
 ### Bug Fixes
-- Fixed the command line help not displaying `--checkpoint-sync-url` as an option.
-- Fixed bug preventing node to startup when using `--exit-when-no-validator-keys-enabled` even with keys present
+- Fixed the command line help not displaying `--checkpoint-sync-url` as an option. [#7823](https://github.com/Consensys/teku/issues/7823)
+- Fixed bug preventing node to startup when using `--exit-when-no-validator-keys-enabled` even with keys present. [#7829](https://github.com/Consensys/teku/pull/7829)
+- Fixed bug when node would not start if it failed downloading the deposit snapshot tree from Beacon API [#7827](https://github.com/Consensys/teku/issues/7827). 
diff --git a/...ces/powchain/src/main/java/tech/pegasys/teku/services/powchain/PowchainConfiguration.java b/...ces/powchain/src/main/java/tech/pegasys/teku/services/powchain/PowchainConfiguration.java
@@ -22,7 +22,6 @@
 import org.apache.commons.lang3.StringUtils;
 import tech.pegasys.teku.ethereum.execution.types.Eth1Address;
 import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException;
-import tech.pegasys.teku.infrastructure.http.UrlSanitizer;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
 import tech.pegasys.teku.spec.Spec;
 import tech.pegasys.teku.spec.networks.Eth2Network;
@@ -31,7 +30,6 @@ public class PowchainConfiguration {
   public static final int DEFAULT_ETH1_LOGS_MAX_BLOCK_RANGE = 10_000;
   public static final boolean DEFAULT_USE_MISSING_DEPOSIT_EVENT_LOGGING = false;
   public static final boolean DEFAULT_DEPOSIT_SNAPSHOT_ENABLED = true;
-  public static final String DEPOSIT_SNAPSHOT_URL_PATH = "/eth/v1/beacon/deposit_snapshot";
 
   private final Spec spec;
   private final List<String> eth1Endpoints;
@@ -182,15 +180,6 @@ public Builder depositSnapshotPath(final String depositSnapshotPath) {
       return this;
     }
 
-    public Builder depositSnapshotPathFromCheckpointSyncUrl(final String checkpointSyncUrl) {
-      if (StringUtils.isNotBlank(checkpointSyncUrl)) {
-        this.depositSnapshotPath =
-            Optional.of(UrlSanitizer.appendPath(checkpointSyncUrl, DEPOSIT_SNAPSHOT_URL_PATH));
-        return this;
-      }
-      throw new IllegalStateException("checkpointSyncUrl cannot be null/empty");
-    }
-
     public Builder setDepositSnapshotPathForNetwork(final Optional<Eth2Network> eth2Network) {
       checkNotNull(eth2Network);
       if (eth2Network.isPresent()

diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/DepositOptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/DepositOptions.java
@@ -23,8 +23,6 @@
 
 public class DepositOptions {
 
-  @CommandLine.Spec CommandLine.Model.CommandSpec commandSpec;
-
   @Option(
       names = {"--eth1-endpoints", "--eth1-endpoint"},
       paramLabel = "<NETWORK>",
@@ -72,31 +70,15 @@ public class DepositOptions {
 
   public void configure(final TekuConfiguration.Builder builder) {
     builder.powchain(
-        b -> {
-          b.eth1Endpoints(eth1Endpoints)
-              .eth1LogsMaxBlockRange(eth1LogsMaxBlockRange)
-              .useMissingDepositEventLogging(useMissingDepositEventLogging);
-          handleDepositSnapshotOptions(b);
-        });
+        b ->
+            b.eth1Endpoints(eth1Endpoints)
+                .eth1LogsMaxBlockRange(eth1LogsMaxBlockRange)
+                .useMissingDepositEventLogging(useMissingDepositEventLogging)
+                .depositSnapshotPath(depositSnapshotPath)
+                .depositSnapshotEnabled(parseDepositSnapshotEnabled()));
   }
 
-  private void handleDepositSnapshotOptions(final PowchainConfiguration.Builder b) {
-    final String checkpointSyncUrlOption = "--checkpoint-sync-url";
-    CommandLine.ParseResult parseResult = commandSpec.commandLine().getParseResult();
-    final boolean depositSnapshotPathSet = parseResult.hasMatchedOption("--Xdeposit-snapshot");
-    final boolean depositSnapshotEnabledSet =
-        parseResult.hasMatchedOption("--deposit-snapshot-enabled");
-    final boolean checkpointSyncUrlSet = parseResult.hasMatchedOption(checkpointSyncUrlOption);
-
-    if (depositSnapshotEnabledSet && depositSnapshotEnabled) {
-      b.depositSnapshotPath(depositSnapshotPath).depositSnapshotEnabled(depositSnapshotEnabled);
-    } else if (depositSnapshotPathSet) {
-      b.depositSnapshotPath(depositSnapshotPath).depositSnapshotEnabled(false);
-    } else if (checkpointSyncUrlSet) {
-      final String checkpointSyncUrl = parseResult.matchedOptionValue(checkpointSyncUrlOption, "");
-      b.depositSnapshotPathFromCheckpointSyncUrl(checkpointSyncUrl).depositSnapshotEnabled(false);
-    } else {
-      b.depositSnapshotPath(depositSnapshotPath).depositSnapshotEnabled(depositSnapshotEnabled);
-    }
+  private boolean parseDepositSnapshotEnabled() {
+    return depositSnapshotEnabled;
   }
 }
diff --git a/teku/src/test/java/tech/pegasys/teku/cli/options/DepositOptionsTest.java b/teku/src/test/java/tech/pegasys/teku/cli/options/DepositOptionsTest.java
@@ -18,12 +18,8 @@
 import static tech.pegasys.teku.beacon.pow.DepositSnapshotFileLoader.DEFAULT_SNAPSHOT_RESOURCE_PATHS;
 
 import java.util.List;
-import java.util.Optional;
-import java.util.stream.Stream;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.Arguments;
-import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
 import tech.pegasys.teku.cli.AbstractBeaconNodeCommandTest;
 import tech.pegasys.teku.config.TekuConfiguration;
@@ -125,7 +121,12 @@ public void shouldSetDefaultBundleSnapshotPathForSupportedNetwork(final String n
     final TekuConfiguration config = getTekuConfigurationFromArguments(args);
     assertThat(config.powchain().isDepositSnapshotEnabled()).isTrue();
     assertThat(config.powchain().getDepositSnapshotPath())
-        .contains(getDefaultBundleSnapshotPath(network));
+        .contains(
+            PowchainConfiguration.class
+                .getResource(
+                    DEFAULT_SNAPSHOT_RESOURCE_PATHS.get(
+                        Eth2Network.fromStringLenient(network).get()))
+                .toExternalForm());
   }
 
   @Test
@@ -147,52 +148,4 @@ public void shouldThrowErrorIfSnapshotPathProvidedWithBundleSnapshot() {
         .isInstanceOf(InvalidConfigurationException.class)
         .hasMessage("Use either custom deposit tree snapshot path or snapshot bundle");
   }
-
-  @ParameterizedTest
-  @MethodSource("getDepositSnapshotOptions")
-  public void shouldHandleSnapshotPathAndEnabledConfig(
-      final String[] args, final String expectedPath, final boolean expectedIsEnabled) {
-    final PowchainConfiguration config = getTekuConfigurationFromArguments(args).powchain();
-    assertThat(config.getDepositSnapshotPath()).isEqualTo(Optional.of(expectedPath));
-    assertThat(config.isDepositSnapshotEnabled()).isEqualTo(expectedIsEnabled);
-  }
-
-  public static Stream<Arguments> getDepositSnapshotOptions() {
-    final String[] emptyConfig = {"--network=mainnet"};
-    final String[] depositSnapshotEnabledSet = {
-      "--network=mainnet",
-      "--deposit-snapshot-enabled=true",
-      "--checkpoint-sync-url=http://checkpoint/path/"
-    };
-    final String[] depositSnapshotPathSet = {"--Xdeposit-snapshot=/some/path/"};
-    final String[] depositSnapshotPathSetAndCheckpointSync = {
-      "--Xdeposit-snapshot=/some/path/",
-      "--deposit-snapshot-enabled=false",
-      "--checkpoint-sync-url=http://checkpoint/path/"
-    };
-    final String[] depositSnapshotDisabledAndSync = {
-      "--deposit-snapshot-enabled=false", "--checkpoint-sync-url=http://checkpoint/path/"
-    };
-    final String[] checkpointSyncUrlSet = {"--checkpoint-sync-url=http://checkpoint/path/"};
-    final String defaultBundleSnapshotPath = getDefaultBundleSnapshotPath("mainnet");
-
-    return Stream.of(
-        Arguments.of(emptyConfig, defaultBundleSnapshotPath, true),
-        Arguments.of(depositSnapshotEnabledSet, defaultBundleSnapshotPath, true),
-        Arguments.of(depositSnapshotPathSet, "/some/path/", false),
-        Arguments.of(depositSnapshotPathSetAndCheckpointSync, "/some/path/", false),
-        Arguments.of(
-            depositSnapshotDisabledAndSync,
-            "http://checkpoint/eth/v1/beacon/deposit_snapshot",
-            false),
-        Arguments.of(
-            checkpointSyncUrlSet, "http://checkpoint/eth/v1/beacon/deposit_snapshot", false));
-  }
-
-  private static String getDefaultBundleSnapshotPath(final String network) {
-    return PowchainConfiguration.class
-        .getResource(
-            DEFAULT_SNAPSHOT_RESOURCE_PATHS.get(Eth2Network.fromStringLenient(network).get()))
-        .toExternalForm();
-  }
 }