From e6849a2bc664402786f81d3325229fe04a417274 Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta Date: Thu, 4 Apr 2019 11:27:46 +0200 Subject: [PATCH 01/14] fix PAN-2265 - add `--fast-sync-min-peers` hidden cli option - add `--fast-sync-max-wait-time` hidden cli option - update `FastSyncActions` to handle no timeout behaviour (when timeout set to 0 or no timeout set then we just trigger asynchrounously the waitPeersTask) - update unit tests mock related to sync config builder - add fast sync options in `everything_config.toml` - re enable `PantheonCommandTest.syncModeOptionMustBeUsed` fix PAN-2265 fixes PAN-2265 --- .../eth/sync/fastsync/FastSyncActions.java | 20 +++++- .../pantheon/cli/DefaultCommandValues.java | 2 + .../pegasys/pantheon/cli/PantheonCommand.java | 70 ++++++++++++++----- .../pantheon/cli/CommandTestAbstract.java | 4 ++ .../pantheon/cli/PantheonCommandTest.java | 1 - .../src/test/resources/everything_config.toml | 2 + 6 files changed, 81 insertions(+), 18 deletions(-) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java index 3ecbbd1cd4..d3a4dedca4 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java +++ b/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,25 @@ public CompletableFuture waitForSuitablePeers(final FastSyncState ethContext, syncConfig.getFastSyncMinimumPeerCount(), metricsSystem); final EthScheduler scheduler = ethContext.getScheduler(); + final CompletableFuture 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) { diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/DefaultCommandValues.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/DefaultCommandValues.java index 0403b01533..284ca21313 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/DefaultCommandValues.java +++ b/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 DEFAULT_FAST_SYNC_MAX_WAIT_TIME = 0; + int DEFAULT_FAST_SYNC_MIN_PEER_COUNT = 5; int DEFAULT_MAX_PEERS = 25; static Path getDefaultPantheonDataPath(final Object command) { diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index 9ac00ea716..cfa62ca5d8 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/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; @@ -72,8 +75,8 @@ 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.Arrays; import java.util.Collection; import java.util.List; import java.util.Optional; @@ -221,13 +224,28 @@ void setBootnodes(final List values) { private final Collection 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, possible values are ${COMPLETION-CANDIDATES} (default: ${DEFAULT-VALUE})") + private final Integer fastSyncMinPeerCount = DEFAULT_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, possible values are ${COMPLETION-CANDIDATES} (default: ${DEFAULT-VALUE})") + private final Integer fastSyncMaxWaitTime = DEFAULT_FAST_SYNC_MAX_WAIT_TIME; + @Option( names = {"--network"}, paramLabel = MANDATORY_NETWORK_FORMAT_HELP, @@ -590,12 +608,12 @@ 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", @@ -603,12 +621,15 @@ public void run() { "--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 +679,21 @@ 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.equals(SyncMode.FAST), + 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 +733,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 +767,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 +805,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,12 +918,12 @@ private boolean contractPermissionsEnabled() { private PrivacyParameters privacyParameters() throws IOException { - CommandLineUtils.checkOptionDependencies( + checkOptionDependencies( logger, commandLine, "--privacy-enabled", !isPrivacyEnabled, - Arrays.asList( + asList( "--privacy-url", "--privacy-public-key-file", "--privacy-precompiled-address")); final PrivacyParameters privacyParameters = PrivacyParameters.noPrivacy(); @@ -909,6 +945,8 @@ private PrivacyParameters privacyParameters() throws IOException { private SynchronizerConfiguration buildSyncConfig() { return synchronizerConfigurationBuilder .syncMode(syncMode) + .fastSyncMinimumPeerCount(fastSyncMinPeerCount) + .fastSyncMaximumPeerWaitTime(ofSeconds(fastSyncMaxWaitTime)) .maxTrailingPeers(TrailingPeerRequirements.calculateMaxTrailingPeers(maxPeers)) .build(); } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java index 6300ecd167..366a5bc112 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java +++ b/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,9 @@ 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); diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index 5428a0c699..35523e77aa 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -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() { diff --git a/pantheon/src/test/resources/everything_config.toml b/pantheon/src/test/resources/everything_config.toml index 8e3a4516fe..e456e340bb 100644 --- a/pantheon/src/test/resources/everything_config.toml +++ b/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 From 32b4a59366863dd3aff032f4c35494dfb08a3852 Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta Date: Thu, 4 Apr 2019 15:48:46 +0200 Subject: [PATCH 02/14] fix style gradlew spotlessApply --- .../eth/sync/fastsync/FastSyncActions.java | 23 +++++----- .../pegasys/pantheon/cli/PantheonCommand.java | 42 +++++++++---------- .../pantheon/cli/CommandTestAbstract.java | 3 +- .../pantheon/cli/PantheonCommandTest.java | 1 - 4 files changed, 32 insertions(+), 37 deletions(-) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java index d3a4dedca4..8336cfa3e0 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java @@ -73,20 +73,19 @@ public CompletableFuture waitForSuitablePeers(final FastSyncState final EthScheduler scheduler = ethContext.getScheduler(); final CompletableFuture fastSyncTask; - if(!syncConfig.getFastSyncMaximumPeerWaitTime().isZero()) { + 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 { + 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()) - ); + format( + "Waiting for at least %d peers, no maximum wait time set.", + syncConfig.getFastSyncMinimumPeerCount())); fastSyncTask = scheduler.scheduleServiceTask(waitForPeersTask); } return exceptionallyCompose( diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index cfa62ca5d8..c36713496a 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -75,7 +75,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; @@ -231,19 +230,19 @@ void setBootnodes(final List values) { 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, possible values are ${COMPLETION-CANDIDATES} (default: ${DEFAULT-VALUE})") + hidden = true, + names = {"--fast-sync-min-peers"}, + paramLabel = MANDATORY_INTEGER_FORMAT_HELP, + description = + "Minimum number of peers before starting fast sync, possible values are ${COMPLETION-CANDIDATES} (default: ${DEFAULT-VALUE})") private final Integer fastSyncMinPeerCount = DEFAULT_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, possible values are ${COMPLETION-CANDIDATES} (default: ${DEFAULT-VALUE})") + 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, possible values are ${COMPLETION-CANDIDATES} (default: ${DEFAULT-VALUE})") private final Integer fastSyncMaxWaitTime = DEFAULT_FAST_SYNC_MAX_WAIT_TIME; @Option( @@ -679,19 +678,17 @@ public void run() { } } - private void checkFastSyncOptions() throws ParameterException{ + private void checkFastSyncOptions() throws ParameterException { if (fastSyncMaxWaitTime < 0) { - throw new ParameterException(commandLine, "--fast-sync-max-wait-time must be a positive or zero integer"); + throw new ParameterException( + commandLine, "--fast-sync-max-wait-time must be a positive or zero integer"); } checkOptionDependencies( - logger, - commandLine, - "--sync-mode", - syncMode.equals(SyncMode.FAST), - asList( - "--fast-sync-num-peers", - "--fast-sync-timeout" - )); + logger, + commandLine, + "--sync-mode", + syncMode.equals(SyncMode.FAST), + asList("--fast-sync-num-peers", "--fast-sync-timeout")); } private NetworkName getNetwork() { @@ -923,8 +920,7 @@ private PrivacyParameters privacyParameters() throws IOException { commandLine, "--privacy-enabled", !isPrivacyEnabled, - 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) { diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java index 366a5bc112..ef335e1b33 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java @@ -112,7 +112,8 @@ 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.fastSyncMaximumPeerWaitTime(any(Duration.class))) + .thenReturn(mockSyncConfBuilder); when(mockSyncConfBuilder.build()).thenReturn(mockSyncConf); diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index 35523e77aa..e238da9af7 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -73,7 +73,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; From 157aa3ad0ce7f89eea196485bd3e8ce9a795172a Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta Date: Thu, 4 Apr 2019 17:44:15 +0200 Subject: [PATCH 03/14] fix PR discussion after review - fix coding style convention - add unit tests --- .../pantheon/cli/DefaultCommandValues.java | 4 ++-- .../pegasys/pantheon/cli/PantheonCommand.java | 10 ++++----- .../pantheon/cli/PantheonCommandTest.java | 22 +++++++++++++++++++ 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/DefaultCommandValues.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/DefaultCommandValues.java index 284ca21313..9bbcd26be1 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/DefaultCommandValues.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/DefaultCommandValues.java @@ -55,8 +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 DEFAULT_FAST_SYNC_MAX_WAIT_TIME = 0; - int DEFAULT_FAST_SYNC_MIN_PEER_COUNT = 5; + 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) { diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index c36713496a..082582b49c 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -234,16 +234,16 @@ void setBootnodes(final List values) { names = {"--fast-sync-min-peers"}, paramLabel = MANDATORY_INTEGER_FORMAT_HELP, description = - "Minimum number of peers before starting fast sync, possible values are ${COMPLETION-CANDIDATES} (default: ${DEFAULT-VALUE})") - private final Integer fastSyncMinPeerCount = DEFAULT_FAST_SYNC_MIN_PEER_COUNT; + "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, possible values are ${COMPLETION-CANDIDATES} (default: ${DEFAULT-VALUE})") - private final Integer fastSyncMaxWaitTime = DEFAULT_FAST_SYNC_MAX_WAIT_TIME; + "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"}, @@ -687,7 +687,7 @@ private void checkFastSyncOptions() throws ParameterException { logger, commandLine, "--sync-mode", - syncMode.equals(SyncMode.FAST), + SyncMode.FAST.equals(syncMode), asList("--fast-sync-num-peers", "--fast-sync-timeout")); } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index e238da9af7..7f55e0dfe1 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/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; @@ -1047,6 +1048,27 @@ public void syncModeOptionMustBeUsed() { assertThat(commandErrorOutput.toString()).isEmpty(); } + + @Test + public void fastSyncTimeoutOptionMustBeUsed() { + + parseCommand("--sync-mode", "FAST", "--fast-sync-max-wait-time", "30"); + verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FAST)); + verify(mockSyncConfBuilder).fastSyncMaximumPeerWaitTime(ArgumentMatchers.eq(Duration.ofSeconds(30))); + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + } + + @Test + public void fastSyncMinPeersOptionMustBeUsed() { + + parseCommand("--sync-mode", "FAST", "--fast-sync-min-peers", "5"); + verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FAST)); + verify(mockSyncConfBuilder).fastSyncMinimumPeerCount(ArgumentMatchers.eq(5)); + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + } + @Test public void rpcHttpEnabledPropertyDefaultIsFalse() { parseCommand(); From 4424756819dbd6cbe6afe241b4fd2b70d00f1d49 Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta Date: Thu, 4 Apr 2019 18:28:22 +0200 Subject: [PATCH 04/14] Update PantheonCommandTest.java PR discussion --- .../tech/pegasys/pantheon/cli/PantheonCommandTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index 7f55e0dfe1..44ab69c5de 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -1052,9 +1052,9 @@ public void syncModeOptionMustBeUsed() { @Test public void fastSyncTimeoutOptionMustBeUsed() { - parseCommand("--sync-mode", "FAST", "--fast-sync-max-wait-time", "30"); + parseCommand("--sync-mode", "FAST", "--fast-sync-max-wait-time", "17"); verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FAST)); - verify(mockSyncConfBuilder).fastSyncMaximumPeerWaitTime(ArgumentMatchers.eq(Duration.ofSeconds(30))); + verify(mockSyncConfBuilder).fastSyncMaximumPeerWaitTime(ArgumentMatchers.eq(Duration.ofSeconds(17))); assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()).isEmpty(); } @@ -1062,9 +1062,9 @@ public void fastSyncTimeoutOptionMustBeUsed() { @Test public void fastSyncMinPeersOptionMustBeUsed() { - parseCommand("--sync-mode", "FAST", "--fast-sync-min-peers", "5"); + parseCommand("--sync-mode", "FAST", "--fast-sync-min-peers", "11"); verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FAST)); - verify(mockSyncConfBuilder).fastSyncMinimumPeerCount(ArgumentMatchers.eq(5)); + verify(mockSyncConfBuilder).fastSyncMinimumPeerCount(ArgumentMatchers.eq(11)); assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()).isEmpty(); } From 1b984a40d94f61396a13a680eaaaae25cf1d82cb Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta Date: Thu, 4 Apr 2019 18:43:11 +0200 Subject: [PATCH 05/14] Update PantheonCommandTest.java spotlessApply --- .../java/tech/pegasys/pantheon/cli/PantheonCommandTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index 44ab69c5de..a8942cfbaa 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -1048,13 +1048,13 @@ 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))); + verify(mockSyncConfBuilder) + .fastSyncMaximumPeerWaitTime(ArgumentMatchers.eq(Duration.ofSeconds(17))); assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()).isEmpty(); } From 005ef3390616cc1b15eb90920fc3cf1fa7318843 Mon Sep 17 00:00:00 2001 From: mbaxter Date: Thu, 4 Apr 2019 19:11:24 +0200 Subject: [PATCH 06/14] Update pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java Co-Authored-By: abdelhamidbakhta <45264458+abdelhamidbakhta@users.noreply.github.com> --- .../main/java/tech/pegasys/pantheon/cli/PantheonCommand.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index 082582b49c..c60c412d5d 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -681,7 +681,7 @@ 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"); + commandLine, "--fast-sync-max-wait-time must be greater than or equal to 0"); } checkOptionDependencies( logger, From a5740db6e63dbbb15112028538e1813183e892b1 Mon Sep 17 00:00:00 2001 From: mbaxter Date: Thu, 4 Apr 2019 19:11:44 +0200 Subject: [PATCH 07/14] Update pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java Co-Authored-By: abdelhamidbakhta <45264458+abdelhamidbakhta@users.noreply.github.com> --- .../main/java/tech/pegasys/pantheon/cli/PantheonCommand.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index c60c412d5d..5609b0987b 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -234,7 +234,7 @@ void setBootnodes(final List values) { names = {"--fast-sync-min-peers"}, paramLabel = MANDATORY_INTEGER_FORMAT_HELP, description = - "Minimum number of peers before starting fast sync. (default: ${DEFAULT-VALUE})") + "Minimum number of peers required before starting fast sync. (default: ${DEFAULT-VALUE})") private final Integer fastSyncMinPeerCount = FAST_SYNC_MIN_PEER_COUNT; @Option( From a2435df0fc421571a6ea4757ca76385f6f01a951 Mon Sep 17 00:00:00 2001 From: mbaxter Date: Thu, 4 Apr 2019 19:11:53 +0200 Subject: [PATCH 08/14] Update pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java Co-Authored-By: abdelhamidbakhta <45264458+abdelhamidbakhta@users.noreply.github.com> --- .../main/java/tech/pegasys/pantheon/cli/PantheonCommand.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index 5609b0987b..cd555b23fc 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -242,7 +242,7 @@ void setBootnodes(final List values) { 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})") + "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( From 1a7338cac205bce192dcb1aaa3bd29c0234cf793 Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta Date: Thu, 4 Apr 2019 19:19:14 +0200 Subject: [PATCH 09/14] fix PR discussion --- .../ethereum/eth/sync/SynchronizerConfiguration.java | 2 +- .../ethereum/eth/sync/fastsync/FastSyncActions.java | 4 ++-- .../java/tech/pegasys/pantheon/cli/PantheonCommand.java | 2 +- .../tech/pegasys/pantheon/cli/PantheonCommandTest.java | 8 ++++++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java index 0b30180f44..497d838f4c 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java @@ -27,7 +27,7 @@ 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.ofMinutes(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; diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java index 8336cfa3e0..aa40dcf1b9 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java @@ -74,7 +74,7 @@ public CompletableFuture waitForSuitablePeers(final FastSyncState final EthScheduler scheduler = ethContext.getScheduler(); final CompletableFuture fastSyncTask; if (!syncConfig.getFastSyncMaximumPeerWaitTime().isZero()) { - LOG.info( + LOG.debug( format( "Waiting for at least %d peers, maximum wait time set to %s.", syncConfig.getFastSyncMinimumPeerCount(), @@ -82,7 +82,7 @@ public CompletableFuture waitForSuitablePeers(final FastSyncState fastSyncTask = scheduler.timeout(waitForPeersTask, syncConfig.getFastSyncMaximumPeerWaitTime()); } else { - LOG.info( + LOG.debug( format( "Waiting for at least %d peers, no maximum wait time set.", syncConfig.getFastSyncMinimumPeerCount())); diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index cd555b23fc..19c782f560 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -230,7 +230,7 @@ void setBootnodes(final List values) { private final SyncMode syncMode = DEFAULT_SYNC_MODE; @Option( - hidden = true, + hidden = false, names = {"--fast-sync-min-peers"}, paramLabel = MANDATORY_INTEGER_FORMAT_HELP, description = diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index a8942cfbaa..a3ff88fc94 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -33,6 +33,7 @@ import static tech.pegasys.pantheon.ethereum.jsonrpc.RpcApis.WEB3; import static tech.pegasys.pantheon.ethereum.p2p.config.DiscoveryConfiguration.MAINNET_BOOTSTRAP_NODES; +import org.junit.Ignore; import tech.pegasys.pantheon.PantheonInfo; import tech.pegasys.pantheon.config.GenesisConfigFile; import tech.pegasys.pantheon.ethereum.core.Address; @@ -1036,6 +1037,7 @@ public void maxpeersOptionMustBeUsed() { } @Test + @Ignore public void syncModeOptionMustBeUsed() { parseCommand("--sync-mode", "FAST"); @@ -1049,7 +1051,8 @@ public void syncModeOptionMustBeUsed() { } @Test - public void fastSyncTimeoutOptionMustBeUsed() { + @Ignore + public void parsesValidFastSyncTimeoutOption() { parseCommand("--sync-mode", "FAST", "--fast-sync-max-wait-time", "17"); verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FAST)); @@ -1060,7 +1063,8 @@ public void fastSyncTimeoutOptionMustBeUsed() { } @Test - public void fastSyncMinPeersOptionMustBeUsed() { + @Ignore + public void parsesValidFastSyncMinPeersOption() { parseCommand("--sync-mode", "FAST", "--fast-sync-min-peers", "11"); verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FAST)); From 1e4f2b8f537806dce3b1f9f9097a4b4091ec7bd2 Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta Date: Thu, 4 Apr 2019 19:27:00 +0200 Subject: [PATCH 10/14] update PR discussion --- .../eth/sync/fastsync/FastSyncActions.java | 13 ++++----- .../pegasys/pantheon/cli/PantheonCommand.java | 28 ++++++++----------- .../pantheon/cli/PantheonCommandTest.java | 2 +- 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java index aa40dcf1b9..75dfb3c1e3 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java @@ -12,7 +12,6 @@ */ 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; @@ -75,17 +74,15 @@ public CompletableFuture waitForSuitablePeers(final FastSyncState final CompletableFuture fastSyncTask; if (!syncConfig.getFastSyncMaximumPeerWaitTime().isZero()) { LOG.debug( - format( - "Waiting for at least %d peers, maximum wait time set to %s.", - syncConfig.getFastSyncMinimumPeerCount(), - syncConfig.getFastSyncMaximumPeerWaitTime().toString())); + "Waiting for at least {} peers, maximum wait time set to {}.", + syncConfig.getFastSyncMinimumPeerCount(), + syncConfig.getFastSyncMaximumPeerWaitTime().toString()); fastSyncTask = scheduler.timeout(waitForPeersTask, syncConfig.getFastSyncMaximumPeerWaitTime()); } else { LOG.debug( - format( - "Waiting for at least %d peers, no maximum wait time set.", - syncConfig.getFastSyncMinimumPeerCount())); + "Waiting for at least {} peers, no maximum wait time set.", + syncConfig.getFastSyncMinimumPeerCount()); fastSyncTask = scheduler.scheduleServiceTask(waitForPeersTask); } return exceptionallyCompose( diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index 19c782f560..e217c2b0a4 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -14,7 +14,6 @@ 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; @@ -75,6 +74,7 @@ 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; @@ -628,7 +628,16 @@ public void run() { asList("--miner-coinbase", "--min-gas-price", "--miner-extra-data")); // Check that fast sync options are able to work or send an error - checkFastSyncOptions(); + 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-num-peers", "--fast-sync-timeout")); //noinspection ConstantConditions if (isMiningEnabled && coinbase == null) { @@ -678,19 +687,6 @@ public void run() { } } - private void checkFastSyncOptions() throws ParameterException { - 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-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; @@ -942,7 +938,7 @@ private SynchronizerConfiguration buildSyncConfig() { return synchronizerConfigurationBuilder .syncMode(syncMode) .fastSyncMinimumPeerCount(fastSyncMinPeerCount) - .fastSyncMaximumPeerWaitTime(ofSeconds(fastSyncMaxWaitTime)) + .fastSyncMaximumPeerWaitTime(Duration.ofSeconds(fastSyncMaxWaitTime)) .maxTrailingPeers(TrailingPeerRequirements.calculateMaxTrailingPeers(maxPeers)) .build(); } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index a3ff88fc94..514f9c96ef 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -33,7 +33,6 @@ import static tech.pegasys.pantheon.ethereum.jsonrpc.RpcApis.WEB3; import static tech.pegasys.pantheon.ethereum.p2p.config.DiscoveryConfiguration.MAINNET_BOOTSTRAP_NODES; -import org.junit.Ignore; import tech.pegasys.pantheon.PantheonInfo; import tech.pegasys.pantheon.config.GenesisConfigFile; import tech.pegasys.pantheon.ethereum.core.Address; @@ -75,6 +74,7 @@ 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; From 79a1a7778b95c818e83aa489088d262dc48b2ff5 Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta Date: Thu, 4 Apr 2019 22:27:11 +0200 Subject: [PATCH 11/14] add error cases --- .../tech/pegasys/pantheon/cli/PantheonCommand.java | 3 ++- .../pegasys/pantheon/cli/PantheonCommandTest.java | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index e217c2b0a4..04dff91cf9 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -223,6 +223,7 @@ void setBootnodes(final List values) { private final Collection bannedNodeIds = new ArrayList<>(); @Option( + hidden = true, names = {"--sync-mode"}, paramLabel = MANDATORY_MODE_FORMAT_HELP, description = @@ -230,7 +231,7 @@ void setBootnodes(final List values) { private final SyncMode syncMode = DEFAULT_SYNC_MODE; @Option( - hidden = false, + hidden = true, names = {"--fast-sync-min-peers"}, paramLabel = MANDATORY_INTEGER_FORMAT_HELP, description = diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index 514f9c96ef..0b3f068b50 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -1051,7 +1051,6 @@ public void syncModeOptionMustBeUsed() { } @Test - @Ignore public void parsesValidFastSyncTimeoutOption() { parseCommand("--sync-mode", "FAST", "--fast-sync-max-wait-time", "17"); @@ -1063,7 +1062,18 @@ public void parsesValidFastSyncTimeoutOption() { } @Test - @Ignore + public void parsesInvalidFastSyncTimeoutOptionShouldFail() { + parseCommand("--sync-mode", "FAST", "--fast-sync-max-wait-time", "-1"); + + verifyZeroInteractions(mockRunnerBuilder); + + assertThat(commandOutput.toString()).isEmpty(); + // PicoCLI uses longest option name for message when option has multiple names, so here plural. + assertThat(commandErrorOutput.toString()) + .contains("--fast-sync-max-wait-time must be greater than or equal to 0"); + } + + @Test public void parsesValidFastSyncMinPeersOption() { parseCommand("--sync-mode", "FAST", "--fast-sync-min-peers", "11"); From ae970d90800b23c4a48dcab05837d85fbdf2ecd0 Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta Date: Thu, 4 Apr 2019 23:40:55 +0200 Subject: [PATCH 12/14] Update SynchronizerConfiguration.java --- .../pantheon/ethereum/eth/sync/SynchronizerConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java index 497d838f4c..0b30180f44 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java @@ -27,7 +27,7 @@ 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 Duration DEFAULT_FAST_SYNC_MAXIMUM_PEER_WAIT_TIME = Duration.ofMinutes(5); 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; From ae3febcc2390b813fc1c9005fc7ce746d249349f Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta Date: Fri, 5 Apr 2019 00:04:29 +0200 Subject: [PATCH 13/14] change FastSyncActionsTest setup set timeout to 5 minutes --- .../pantheon/ethereum/eth/sync/SynchronizerConfiguration.java | 2 +- .../ethereum/eth/sync/fastsync/FastSyncActionsTest.java | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java index 0b30180f44..497d838f4c 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java @@ -27,7 +27,7 @@ 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.ofMinutes(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; diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActionsTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActionsTest.java index 1d9450c230..95a3466094 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActionsTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActionsTest.java @@ -38,6 +38,7 @@ 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; @@ -50,6 +51,7 @@ public class FastSyncActionsTest { private final SynchronizerConfiguration syncConfig = new SynchronizerConfiguration.Builder() .syncMode(SyncMode.FAST) + .fastSyncMaximumPeerWaitTime(Duration.ofMinutes(5)) .fastSyncPivotDistance(1000) .build(); From c6cda82c01a3dc957afe4440fa84518c26c78280 Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta Date: Fri, 5 Apr 2019 09:12:42 +0200 Subject: [PATCH 14/14] fix PR discussion add unit tests --- .../pantheon/cli/PantheonCommandTest.java | 23 +++++++++++++++---- .../src/test/resources/complete_config.toml | 2 ++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index 0b3f068b50..5cde1c4afa 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -307,8 +307,10 @@ public void overrideDefaultValuesIfKeyIsPresentInConfigFile() throws IOException verify(mockControllerBuilder).homePath(eq(Paths.get("~/pantheondata").toAbsolutePath())); verify(mockControllerBuilder).ethNetworkConfig(eq(networkConfig)); - // TODO: Re-enable as per NC-1057/NC-1681 - // verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FAST)); + verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FAST)); + verify(mockSyncConfBuilder).fastSyncMinimumPeerCount(ArgumentMatchers.eq(13)); + verify(mockSyncConfBuilder) + .fastSyncMaximumPeerWaitTime(ArgumentMatchers.eq(Duration.ofSeconds(57))); assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()).isEmpty(); @@ -609,8 +611,10 @@ public void noOverrideDefaultValuesIfKeyIsNotPresentInConfigFile() throws IOExce .maxPendingTransactions(eq(PendingTransactions.MAX_PENDING_TRANSACTIONS)); verify(mockControllerBuilder).build(); - // TODO: Re-enable as per NC-1057/NC-1681 - // verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FULL)); + verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FULL)); + verify(mockSyncConfBuilder).fastSyncMinimumPeerCount(ArgumentMatchers.eq(5)); + verify(mockSyncConfBuilder) + .fastSyncMaximumPeerWaitTime(ArgumentMatchers.eq(Duration.ofSeconds(0))); assertThat(commandErrorOutput.toString()).isEmpty(); @@ -1068,7 +1072,6 @@ public void parsesInvalidFastSyncTimeoutOptionShouldFail() { verifyZeroInteractions(mockRunnerBuilder); assertThat(commandOutput.toString()).isEmpty(); - // PicoCLI uses longest option name for message when option has multiple names, so here plural. assertThat(commandErrorOutput.toString()) .contains("--fast-sync-max-wait-time must be greater than or equal to 0"); } @@ -1083,6 +1086,16 @@ public void parsesValidFastSyncMinPeersOption() { assertThat(commandErrorOutput.toString()).isEmpty(); } + @Test + public void parsesInvalidFastSyncMinPeersOptionWrongFormatShouldFail() { + + parseCommand("--sync-mode", "FAST", "--fast-sync-min-peers", "ten"); + verifyZeroInteractions(mockRunnerBuilder); + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()) + .contains("Invalid value for option '--fast-sync-min-peers': 'ten' is not an int"); + } + @Test public void rpcHttpEnabledPropertyDefaultIsFalse() { parseCommand(); diff --git a/pantheon/src/test/resources/complete_config.toml b/pantheon/src/test/resources/complete_config.toml index 9b08c300be..22300b0372 100644 --- a/pantheon/src/test/resources/complete_config.toml +++ b/pantheon/src/test/resources/complete_config.toml @@ -27,6 +27,8 @@ metrics-port=309 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