[PAN-2265] Expose fast-sync options on command line

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

@@ -12,6 +12,7 @@
  */
 package tech.pegasys.pantheon.ethereum.eth.sync.fastsync;
 
+import static java.lang.String.format;
 import static java.util.concurrent.CompletableFuture.completedFuture;
 import static tech.pegasys.pantheon.ethereum.eth.sync.fastsync.FastSyncError.CHAIN_TOO_SHORT;
 import static tech.pegasys.pantheon.util.FutureUtils.completedExceptionally;
@@ -71,8 +72,24 @@ public CompletableFuture<FastSyncState> waitForSuitablePeers(final FastSyncState
             ethContext, syncConfig.getFastSyncMinimumPeerCount(), metricsSystem);
 
     final EthScheduler scheduler = ethContext.getScheduler();
+    final CompletableFuture<Void> fastSyncTask;
+    if (!syncConfig.getFastSyncMaximumPeerWaitTime().isZero()) {
+      LOG.info(
+          format(
+              "Waiting for at least %d peers, maximum wait time set to %s.",
+              syncConfig.getFastSyncMinimumPeerCount(),
+              syncConfig.getFastSyncMaximumPeerWaitTime().toString()));
+      fastSyncTask =
+          scheduler.timeout(waitForPeersTask, syncConfig.getFastSyncMaximumPeerWaitTime());
+    } else {
+      LOG.info(
+          format(
+              "Waiting for at least %d peers, no maximum wait time set.",
+              syncConfig.getFastSyncMinimumPeerCount()));
+      fastSyncTask = scheduler.scheduleServiceTask(waitForPeersTask);
+    }
     return exceptionallyCompose(
-            scheduler.timeout(waitForPeersTask, syncConfig.getFastSyncMaximumPeerWaitTime()),
+            fastSyncTask,
             error -> {
               if (ExceptionUtils.rootCause(error) instanceof TimeoutException) {
                 if (ethContext.getEthPeers().availablePeerCount() > 0) {

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

@@ -55,6 +55,8 @@ public interface DefaultCommandValues {
   // Default should be FAST for the next release
   // but we use FULL for the moment as Fast is still in progress
   SyncMode DEFAULT_SYNC_MODE = SyncMode.FULL;
+  int FAST_SYNC_MAX_WAIT_TIME = 0;
+  int FAST_SYNC_MIN_PEER_COUNT = 5;
   int DEFAULT_MAX_PEERS = 25;
 
   static Path getDefaultPantheonDataPath(final Object command) {

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

@@ -14,6 +14,9 @@
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.time.Duration.ofSeconds;
+import static java.util.Arrays.asList;
+import static tech.pegasys.pantheon.cli.CommandLineUtils.checkOptionDependencies;
 import static tech.pegasys.pantheon.cli.DefaultCommandValues.getDefaultPantheonDataPath;
 import static tech.pegasys.pantheon.cli.NetworkName.MAINNET;
 import static tech.pegasys.pantheon.ethereum.jsonrpc.JsonRpcConfiguration.DEFAULT_JSON_RPC_PORT;
@@ -73,7 +76,6 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
@@ -221,13 +223,28 @@ void setBootnodes(final List<String> values) {
   private final Collection<String> bannedNodeIds = new ArrayList<>();
 
   @Option(
-      hidden = true,
       names = {"--sync-mode"},
       paramLabel = MANDATORY_MODE_FORMAT_HELP,
       description =
           "Synchronization mode, possible values are ${COMPLETION-CANDIDATES} (default: ${DEFAULT-VALUE})")
   private final SyncMode syncMode = DEFAULT_SYNC_MODE;
 
+  @Option(
+      hidden = true,
+      names = {"--fast-sync-min-peers"},
+      paramLabel = MANDATORY_INTEGER_FORMAT_HELP,
+      description =
+          "Minimum number of peers 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 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,
@@ -590,25 +607,28 @@ public void run() {
     }
 
     // Check that P2P options are able to work or send an error
-    CommandLineUtils.checkOptionDependencies(
+    checkOptionDependencies(
         logger,
         commandLine,
         "--p2p-enabled",
         !p2pEnabled,
-        Arrays.asList(
+        asList(
             "--bootnodes",
             "--discovery-enabled",
             "--max-peers",
             "--banned-node-id",
             "--banned-node-ids"));
 
     // Check that mining options are able to work or send an error
-    CommandLineUtils.checkOptionDependencies(
+    checkOptionDependencies(
         logger,
         commandLine,
         "--miner-enabled",
         !isMiningEnabled,
-        Arrays.asList("--miner-coinbase", "--min-gas-price", "--miner-extra-data"));
+        asList("--miner-coinbase", "--min-gas-price", "--miner-extra-data"));
+
+    // Check that fast sync options are able to work or send an error
+    checkFastSyncOptions();
 
     //noinspection ConstantConditions
     if (isMiningEnabled && coinbase == null) {
@@ -658,6 +678,19 @@ public void run() {
     }
   }
 
+  private void checkFastSyncOptions() throws ParameterException {
+    if (fastSyncMaxWaitTime < 0) {
+      throw new ParameterException(
+          commandLine, "--fast-sync-max-wait-time must be a positive or zero integer");
+    }
+    checkOptionDependencies(
+        logger,
+        commandLine,
+        "--sync-mode",
+        SyncMode.FAST.equals(syncMode),
+        asList("--fast-sync-num-peers", "--fast-sync-timeout"));
+  }
+
   private NetworkName getNetwork() {
     //noinspection ConstantConditions network is not always null but injected by PicoCLI if used
     return network == null ? MAINNET : network;
@@ -697,12 +730,12 @@ PantheonController<?> buildController() {
 
   private JsonRpcConfiguration jsonRpcConfiguration() {
 
-    CommandLineUtils.checkOptionDependencies(
+    checkOptionDependencies(
         logger,
         commandLine,
         "--rpc-http-enabled",
         !isRpcHttpEnabled,
-        Arrays.asList(
+        asList(
             "--rpc-http-api",
             "--rpc-http-apis",
             "--rpc-http-cors-origins",
@@ -731,12 +764,12 @@ private JsonRpcConfiguration jsonRpcConfiguration() {
 
   private WebSocketConfiguration webSocketConfiguration() {
 
-    CommandLineUtils.checkOptionDependencies(
+    checkOptionDependencies(
         logger,
         commandLine,
         "--rpc-ws-enabled",
         !isRpcWsEnabled,
-        Arrays.asList(
+        asList(
             "--rpc-ws-api",
             "--rpc-ws-apis",
             "--rpc-ws-host",
@@ -769,19 +802,19 @@ MetricsConfiguration metricsConfiguration() {
               + "time.  Please refer to CLI reference for more details about this constraint.");
     }
 
-    CommandLineUtils.checkOptionDependencies(
+    checkOptionDependencies(
         logger,
         commandLine,
         "--metrics-enabled",
         !isMetricsEnabled,
-        Arrays.asList("--metrics-host", "--metrics-port"));
+        asList("--metrics-host", "--metrics-port"));
 
-    CommandLineUtils.checkOptionDependencies(
+    checkOptionDependencies(
         logger,
         commandLine,
         "--metrics-push-enabled",
         !isMetricsPushEnabled,
-        Arrays.asList(
+        asList(
             "--metrics-push-host",
             "--metrics-push-port",
             "--metrics-push-interval",
@@ -882,13 +915,12 @@ private boolean contractPermissionsEnabled() {
 
   private PrivacyParameters privacyParameters() throws IOException {
 
-    CommandLineUtils.checkOptionDependencies(
+    checkOptionDependencies(
         logger,
         commandLine,
         "--privacy-enabled",
         !isPrivacyEnabled,
-        Arrays.asList(
-            "--privacy-url", "--privacy-public-key-file", "--privacy-precompiled-address"));
+        asList("--privacy-url", "--privacy-public-key-file", "--privacy-precompiled-address"));
 
     final PrivacyParameters privacyParameters = PrivacyParameters.noPrivacy();
     if (isPrivacyEnabled) {
@@ -909,6 +941,8 @@ private PrivacyParameters privacyParameters() throws IOException {
   private SynchronizerConfiguration buildSyncConfig() {
     return synchronizerConfigurationBuilder
         .syncMode(syncMode)
+        .fastSyncMinimumPeerCount(fastSyncMinPeerCount)
+        .fastSyncMaximumPeerWaitTime(ofSeconds(fastSyncMaxWaitTime))
         .maxTrailingPeers(TrailingPeerRequirements.calculateMaxTrailingPeers(maxPeers))
         .build();
   }

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

@@ -36,6 +36,7 @@
 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;
@@ -110,6 +111,10 @@ 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(mockRunnerBuilder.vertx(any())).thenReturn(mockRunnerBuilder);

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

@@ -59,6 +59,7 @@
 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;
@@ -73,7 +74,6 @@
 import net.consensys.cava.toml.Toml;
 import net.consensys.cava.toml.TomlParseResult;
 import org.apache.commons.text.StringEscapeUtils;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -1035,7 +1035,6 @@ public void maxpeersOptionMustBeUsed() {
     assertThat(commandErrorOutput.toString()).isEmpty();
   }
 
-  @Ignore("Ignored as we only have one mode available for now. See NC-1057/NC-1681")
   @Test
   public void syncModeOptionMustBeUsed() {
 
@@ -1049,6 +1048,27 @@ public void syncModeOptionMustBeUsed() {
     assertThat(commandErrorOutput.toString()).isEmpty();
   }
 
+  @Test
+  public void fastSyncTimeoutOptionMustBeUsed() {
+
+    parseCommand("--sync-mode", "FAST", "--fast-sync-max-wait-time", "17");
+    verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FAST));
+    verify(mockSyncConfBuilder)
+        .fastSyncMaximumPeerWaitTime(ArgumentMatchers.eq(Duration.ofSeconds(17)));
+    assertThat(commandOutput.toString()).isEmpty();
+    assertThat(commandErrorOutput.toString()).isEmpty();
+  }
+
+  @Test
+  public void fastSyncMinPeersOptionMustBeUsed() {
+
+    parseCommand("--sync-mode", "FAST", "--fast-sync-min-peers", "11");
+    verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FAST));
+    verify(mockSyncConfBuilder).fastSyncMinimumPeerCount(ArgumentMatchers.eq(11));
+    assertThat(commandOutput.toString()).isEmpty();
+    assertThat(commandErrorOutput.toString()).isEmpty();
+  }
+
   @Test
   public void rpcHttpEnabledPropertyDefaultIsFalse() {
     parseCommand();

pantheon/src/test/resources/everything_config.toml

@@ -32,6 +32,8 @@ host-whitelist=["all"]
 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