Remove --fast-sync-wait-time option (PegaSysEng#1297)

ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java

@@ -16,7 +16,6 @@
 
 import tech.pegasys.pantheon.util.uint.UInt256;
 
-import java.time.Duration;
 import java.util.Iterator;
 import java.util.concurrent.TimeUnit;
 
@@ -30,7 +29,6 @@ public class SynchronizerConfiguration {
   private static final int DEFAULT_PIVOT_DISTANCE_FROM_HEAD = 50;
   private static final float DEFAULT_FULL_VALIDATION_RATE = .1f;
   private static final int DEFAULT_FAST_SYNC_MINIMUM_PEERS = 5;
-  private static final Duration DEFAULT_FAST_SYNC_MAXIMUM_PEER_WAIT_TIME = Duration.ofSeconds(0);
   private static final int DEFAULT_WORLD_STATE_HASH_COUNT_PER_REQUEST = 384;
   private static final int DEFAULT_WORLD_STATE_REQUEST_PARALLELISM = 10;
   private static final int DEFAULT_WORLD_STATE_MAX_REQUESTS_WITHOUT_PROGRESS = 1000;
@@ -41,7 +39,6 @@ public class SynchronizerConfiguration {
   private final int fastSyncPivotDistance;
   private final float fastSyncFullValidationRate;
   private final int fastSyncMinimumPeerCount;
-  private final Duration fastSyncMaximumPeerWaitTime;
   private final int worldStateHashCountPerRequest;
   private final int worldStateRequestParallelism;
   private final int worldStateMaxRequestsWithoutProgress;
@@ -69,7 +66,6 @@ private SynchronizerConfiguration(
       final int fastSyncPivotDistance,
       final float fastSyncFullValidationRate,
       final int fastSyncMinimumPeerCount,
-      final Duration fastSyncMaximumPeerWaitTime,
       final int worldStateHashCountPerRequest,
       final int worldStateRequestParallelism,
       final int worldStateMaxRequestsWithoutProgress,
@@ -89,7 +85,6 @@ private SynchronizerConfiguration(
     this.fastSyncPivotDistance = fastSyncPivotDistance;
     this.fastSyncFullValidationRate = fastSyncFullValidationRate;
     this.fastSyncMinimumPeerCount = fastSyncMinimumPeerCount;
-    this.fastSyncMaximumPeerWaitTime = fastSyncMaximumPeerWaitTime;
     this.worldStateHashCountPerRequest = worldStateHashCountPerRequest;
     this.worldStateRequestParallelism = worldStateRequestParallelism;
     this.worldStateMaxRequestsWithoutProgress = worldStateMaxRequestsWithoutProgress;
@@ -192,10 +187,6 @@ public int getFastSyncMinimumPeerCount() {
     return fastSyncMinimumPeerCount;
   }
 
-  public Duration getFastSyncMaximumPeerWaitTime() {
-    return fastSyncMaximumPeerWaitTime;
-  }
-
   public int getWorldStateHashCountPerRequest() {
     return worldStateHashCountPerRequest;
   }
@@ -219,7 +210,6 @@ public int getMaxTrailingPeers() {
   public static class Builder {
     private SyncMode syncMode = SyncMode.FULL;
     private int fastSyncMinimumPeerCount = DEFAULT_FAST_SYNC_MINIMUM_PEERS;
-    private Duration fastSyncMaximumPeerWaitTime = DEFAULT_FAST_SYNC_MAXIMUM_PEER_WAIT_TIME;
     private int maxTrailingPeers = Integer.MAX_VALUE;
 
     @CommandLine.Option(
@@ -465,11 +455,6 @@ public Builder worldStateMaxRequestsWithoutProgress(
       return this;
     }
 
-    public Builder fastSyncMaximumPeerWaitTime(final Duration fastSyncMaximumPeerWaitTime) {
-      this.fastSyncMaximumPeerWaitTime = fastSyncMaximumPeerWaitTime;
-      return this;
-    }
-
     public Builder worldStateMinMillisBeforeStalling(final long worldStateMinMillisBeforeStalling) {
       this.worldStateMinMillisBeforeStalling = worldStateMinMillisBeforeStalling;
       return this;
@@ -485,7 +470,6 @@ public SynchronizerConfiguration build() {
           fastSyncPivotDistance,
           fastSyncFullValidationRate,
           fastSyncMinimumPeerCount,
-          fastSyncMaximumPeerWaitTime,
           worldStateHashCountPerRequest,
           worldStateRequestParallelism,
           worldStateMaxRequestsWithoutProgress,

ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java

@@ -20,7 +20,6 @@
 import tech.pegasys.pantheon.ethereum.ProtocolContext;
 import tech.pegasys.pantheon.ethereum.core.BlockHeader;
 import tech.pegasys.pantheon.ethereum.eth.manager.EthContext;
-import tech.pegasys.pantheon.ethereum.eth.manager.EthScheduler;
 import tech.pegasys.pantheon.ethereum.eth.manager.task.WaitForPeersTask;
 import tech.pegasys.pantheon.ethereum.eth.sync.ChainDownloader;
 import tech.pegasys.pantheon.ethereum.eth.sync.SynchronizerConfiguration;
@@ -70,40 +69,10 @@ public CompletableFuture<FastSyncState> waitForSuitablePeers(final FastSyncState
         WaitForPeersTask.create(
             ethContext, syncConfig.getFastSyncMinimumPeerCount(), metricsSystem);
 
-    final EthScheduler scheduler = ethContext.getScheduler();
-    final CompletableFuture<Void> fastSyncTask;
-    if (!syncConfig.getFastSyncMaximumPeerWaitTime().isZero()) {
-      LOG.debug(
-          "Waiting for at least {} peers, maximum wait time set to {}.",
-          syncConfig.getFastSyncMinimumPeerCount(),
-          syncConfig.getFastSyncMaximumPeerWaitTime().toString());
-      fastSyncTask =
-          scheduler.timeout(waitForPeersTask, syncConfig.getFastSyncMaximumPeerWaitTime());
-    } else {
-      LOG.debug(
-          "Waiting for at least {} peers, no maximum wait time set.",
-          syncConfig.getFastSyncMinimumPeerCount());
-      fastSyncTask = scheduler.scheduleServiceTask(waitForPeersTask);
-    }
-    return exceptionallyCompose(
-            fastSyncTask,
-            error -> {
-              if (ExceptionUtils.rootCause(error) instanceof TimeoutException) {
-                if (ethContext.getEthPeers().availablePeerCount() > 0) {
-                  LOG.warn(
-                      "Fast sync timed out before minimum peer count was reached. Continuing with reduced peers.");
-                  return completedFuture(null);
-                } else {
-                  LOG.warn(
-                      "Maximum wait time for fast sync reached but no peers available. Continuing to wait for any available peer.");
-                  return waitForAnyPeer();
-                }
-              } else if (error != null) {
-                LOG.error("Failed to find peers for fast sync", error);
-                return completedExceptionally(error);
-              }
-              return null;
-            })
+    LOG.debug("Waiting for at least {} peers.", syncConfig.getFastSyncMinimumPeerCount());
+    return ethContext
+        .getScheduler()
+        .scheduleServiceTask(waitForPeersTask)
         .thenApply(successfulWaitResult -> fastSyncState);
   }
 

ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActionsTest.java

@@ -38,7 +38,6 @@
 import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem;
 import tech.pegasys.pantheon.util.uint.UInt256;
 
-import java.time.Duration;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -51,7 +50,6 @@ public class FastSyncActionsTest {
   private final SynchronizerConfiguration syncConfig =
       new SynchronizerConfiguration.Builder()
           .syncMode(SyncMode.FAST)
-          .fastSyncMaximumPeerWaitTime(Duration.ofMinutes(5))
           .fastSyncPivotDistance(1000)
           .build();
 
@@ -94,25 +92,6 @@ public void waitForPeersShouldSucceedIfEnoughPeersAreFound() {
     assertThat(result).isCompletedWithValue(EMPTY_SYNC_STATE);
   }
 
-  @Test
-  public void waitForPeersShouldReportSuccessWhenTimeLimitReachedAndAPeerIsAvailable() {
-    EthProtocolManagerTestUtil.createPeer(ethProtocolManager);
-    timeoutCount.set(Integer.MAX_VALUE);
-    assertThat(fastSyncActions.waitForSuitablePeers(EMPTY_SYNC_STATE))
-        .isCompletedWithValue(EMPTY_SYNC_STATE);
-  }
-
-  @Test
-  public void waitForPeersShouldContinueWaitingUntilAtLeastOnePeerIsAvailable() {
-    timeoutCount.set(1);
-    final CompletableFuture<FastSyncState> result =
-        fastSyncActions.waitForSuitablePeers(EMPTY_SYNC_STATE);
-    assertThat(result).isNotCompleted();
-
-    EthProtocolManagerTestUtil.createPeer(ethProtocolManager);
-    assertThat(result).isCompletedWithValue(EMPTY_SYNC_STATE);
-  }
-
   @Test
   public void waitForPeersShouldOnlyRequireOnePeerWhenPivotBlockIsAlreadySelected() {
     final BlockHeader pivotHeader = new BlockHeaderTestFixture().number(1024).buildHeader();

pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java

@@ -15,6 +15,7 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Arrays.asList;
+import static java.util.Collections.singletonList;
 import static tech.pegasys.pantheon.cli.CommandLineUtils.checkOptionDependencies;
 import static tech.pegasys.pantheon.cli.DefaultCommandValues.getDefaultPantheonDataPath;
 import static tech.pegasys.pantheon.cli.NetworkName.MAINNET;
@@ -79,7 +80,6 @@
 import java.net.URI;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -219,14 +219,6 @@ void setBootnodes(final List<String> values) {
           "Minimum number of peers required before starting fast sync. (default: ${DEFAULT-VALUE})")
   private final Integer fastSyncMinPeerCount = FAST_SYNC_MIN_PEER_COUNT;
 
-  @Option(
-      hidden = true,
-      names = {"--fast-sync-max-wait-time"},
-      paramLabel = MANDATORY_INTEGER_FORMAT_HELP,
-      description =
-          "Maximum time to wait for the required number of peers before starting fast sync, expressed in seconds, 0 means no timeout (default: ${DEFAULT-VALUE})")
-  private final Integer fastSyncMaxWaitTime = FAST_SYNC_MAX_WAIT_TIME;
-
   @Option(
       names = {"--network"},
       paramLabel = MANDATORY_NETWORK_FORMAT_HELP,
@@ -626,17 +618,12 @@ public void run() {
         !isMiningEnabled,
         asList("--miner-coinbase", "--min-gas-price", "--miner-extra-data"));
 
-    // Check that fast sync options are able to work or send an error
-    if (fastSyncMaxWaitTime < 0) {
-      throw new ParameterException(
-          commandLine, "--fast-sync-max-wait-time must be greater than or equal to 0");
-    }
     checkOptionDependencies(
         logger,
         commandLine,
         "--sync-mode",
         !SyncMode.FAST.equals(syncMode),
-        asList("--fast-sync-min-peers", "--fast-sync-max-wait-time"));
+        singletonList("--fast-sync-min-peers"));
 
     //noinspection ConstantConditions
     if (isMiningEnabled && coinbase == null) {
@@ -940,7 +927,6 @@ private SynchronizerConfiguration buildSyncConfig() {
     return synchronizerConfigurationBuilder
         .syncMode(syncMode)
         .fastSyncMinimumPeerCount(fastSyncMinPeerCount)
-        .fastSyncMaximumPeerWaitTime(Duration.ofSeconds(fastSyncMaxWaitTime))
         .maxTrailingPeers(TrailingPeerRequirements.calculateMaxTrailingPeers(maxPeers))
         .build();
   }

pantheon/src/test/java/tech/pegasys/pantheon/RunnerTest.java

@@ -53,7 +53,6 @@
 import java.io.IOException;
 import java.net.InetAddress;
 import java.nio.file.Path;
-import java.time.Duration;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
@@ -169,7 +168,6 @@ private void syncFromGenesis(final SyncMode mode) throws Exception {
               .syncMode(mode)
               .fastSyncPivotDistance(5)
               .fastSyncMinimumPeerCount(1)
-              .fastSyncMaximumPeerWaitTime(Duration.ofSeconds(1))
               .build();
       final Path dataDirBehind = temp.newFolder().toPath();
       final JsonRpcConfiguration behindJsonRpcConfiguration = jsonRpcConfiguration();

pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java

@@ -38,7 +38,6 @@
 import java.io.InputStream;
 import java.io.PrintStream;
 import java.nio.file.Path;
-import java.time.Duration;
 import java.util.Collection;
 
 import org.apache.logging.log4j.LogManager;
@@ -120,8 +119,6 @@ public void initMocks() throws Exception {
     when(mockSyncConfBuilder.syncMode(any())).thenReturn(mockSyncConfBuilder);
     when(mockSyncConfBuilder.maxTrailingPeers(anyInt())).thenReturn(mockSyncConfBuilder);
     when(mockSyncConfBuilder.fastSyncMinimumPeerCount(anyInt())).thenReturn(mockSyncConfBuilder);
-    when(mockSyncConfBuilder.fastSyncMaximumPeerWaitTime(any(Duration.class)))
-        .thenReturn(mockSyncConfBuilder);
     when(mockSyncConfBuilder.build()).thenReturn(mockSyncConf);
 
     when(mockEthereumWireProtocolConfigurationBuilder.build())

pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java

@@ -59,7 +59,6 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.time.Duration;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
@@ -307,7 +306,6 @@ public void overrideDefaultValuesIfKeyIsPresentInConfigFile() throws IOException
 
     verify(mockSyncConfBuilder).syncMode(eq(SyncMode.FAST));
     verify(mockSyncConfBuilder).fastSyncMinimumPeerCount(eq(13));
-    verify(mockSyncConfBuilder).fastSyncMaximumPeerWaitTime(eq(Duration.ofSeconds(57)));
 
     assertThat(commandOutput.toString()).isEmpty();
     assertThat(commandErrorOutput.toString()).isEmpty();
@@ -614,7 +612,6 @@ public void noOverrideDefaultValuesIfKeyIsNotPresentInConfigFile() throws IOExce
 
     verify(mockSyncConfBuilder).syncMode(eq(SyncMode.FULL));
     verify(mockSyncConfBuilder).fastSyncMinimumPeerCount(eq(5));
-    verify(mockSyncConfBuilder).fastSyncMaximumPeerWaitTime(eq(Duration.ofSeconds(0)));
 
     assertThat(commandErrorOutput.toString()).isEmpty();
 
@@ -1063,27 +1060,6 @@ public void helpShouldDisplayFastSyncOptions() {
     assertThat(commandErrorOutput.toString()).isEmpty();
   }
 
-  @Test
-  public void parsesValidFastSyncTimeoutOption() {
-
-    parseCommand("--sync-mode", "FAST", "--fast-sync-max-wait-time", "17");
-    verify(mockSyncConfBuilder).syncMode(eq(SyncMode.FAST));
-    verify(mockSyncConfBuilder).fastSyncMaximumPeerWaitTime(eq(Duration.ofSeconds(17)));
-    assertThat(commandOutput.toString()).isEmpty();
-    assertThat(commandErrorOutput.toString()).isEmpty();
-  }
-
-  @Test
-  public void parsesInvalidFastSyncTimeoutOptionShouldFail() {
-    parseCommand("--sync-mode", "FAST", "--fast-sync-max-wait-time", "-1");
-
-    verifyZeroInteractions(mockRunnerBuilder);
-
-    assertThat(commandOutput.toString()).isEmpty();
-    assertThat(commandErrorOutput.toString())
-        .contains("--fast-sync-max-wait-time must be greater than or equal to 0");
-  }
-
   @Test
   public void parsesValidFastSyncMinPeersOption() {
 
@@ -1268,10 +1244,9 @@ public void rpcHttpOptionsRequiresServiceToBeEnabled() {
 
   @Test
   public void fastSyncOptionsRequiresFastSyncModeToBeSet() {
-    parseCommand("--fast-sync-min-peers", "5", "--fast-sync-max-wait-time", "30");
+    parseCommand("--fast-sync-min-peers", "5");
 
-    verifyOptionsConstraintLoggerCall(
-        "--sync-mode", "--fast-sync-min-peers", "--fast-sync-max-wait-time");
+    verifyOptionsConstraintLoggerCall("--sync-mode", "--fast-sync-min-peers");
 
     assertThat(commandOutput.toString()).isEmpty();
     assertThat(commandErrorOutput.toString()).isEmpty();

pantheon/src/test/resources/complete_config.toml

@@ -28,7 +28,6 @@ genesis-file="~/genesis.json" # Path
 network-id=42
 sync-mode="fast"# should be FAST or FULL (or fast or full)
 fast-sync-min-peers=13
-fast-sync-max-wait-time=57
 ottoman=false # true means using ottoman testnet if genesis file uses iBFT
 
 #mining

pantheon/src/test/resources/everything_config.toml

@@ -33,7 +33,6 @@ network="MAINNET"
 genesis-file="~/genesis.json"
 sync-mode="fast"
 fast-sync-min-peers=5
-fast-sync-max-wait-time=30
 network-id=303
 
 # JSON-RPC