diff --git a/CHANGELOG.md b/CHANGELOG.md index 06c1efcf6ed..f4532a088cf 100644 --- 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/services/powchain/src/main/java/tech/pegasys/teku/services/powchain/PowchainConfiguration.java b/services/powchain/src/main/java/tech/pegasys/teku/services/powchain/PowchainConfiguration.java index 4cf43131116..6686e1d2575 100644 --- a/services/powchain/src/main/java/tech/pegasys/teku/services/powchain/PowchainConfiguration.java +++ b/services/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 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) { 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 index 070182ae845..8ed96775046 100644 --- 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 = "", @@ -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 index db8600c3e5d..d92bfe892af 100644 --- 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 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(); - } }