[PAN-2950] Use java.time.Clock instead of System.currentTimeMillis() (#…

…1747) To allow us to reset the timestamp in the blockchain for Retesteth support we need to pass a Clock to affected APIs and use that instead of the static method System.currentTimeMillis(). The most consistent way to do this that will ensure that the API does not sneak back in is to ban the method via ErrorProne. TestClock.fixed() was altered to return the "now" time of the first time the fixed clock was requested, needed for many header validation tasks validating headers are not from the future.
PegaSysEng · Jul 25, 2019 · 814b36e · 814b36e
1 parent 824d1d2
commit 814b36e
Show file tree

Hide file tree

Showing 104 changed files with 767 additions and 370 deletions.
diff --git a/...c/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java b/...c/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java
@@ -63,14 +63,16 @@ public class ThreadPantheonNodeRunner implements PantheonNodeRunner {
 
   private final Map<Node, PantheonPluginContextImpl> pantheonPluginContextMap = new HashMap<>();
 
-  private PantheonPluginContextImpl buildPluginContext(final PantheonNode node) {
-    PantheonPluginContextImpl pantheonPluginContext = new PantheonPluginContextImpl();
+  private PantheonPluginContextImpl buildPluginContext(
+      final PantheonNode node, final CommandLine commandLine) {
+    final PantheonPluginContextImpl pantheonPluginContext = new PantheonPluginContextImpl();
     final Path pluginsPath = node.homeDirectory().resolve("plugins");
     final File pluginsDirFile = pluginsPath.toFile();
     if (!pluginsDirFile.isDirectory()) {
       pluginsDirFile.mkdirs();
       pluginsDirFile.deleteOnExit();
     }
+    pantheonPluginContext.addService(PicoCLIOptions.class, new PicoCLIOptionsImpl(commandLine));
     System.setProperty("pantheon.plugins.dir", pluginsPath.toString());
     pantheonPluginContext.registerPlugins(pluginsPath);
     return pantheonPluginContext;
@@ -85,8 +87,7 @@ public void startNode(final PantheonNode node) {
 
     final CommandLine commandLine = new CommandLine(CommandSpec.create());
     final PantheonPluginContextImpl pantheonPluginContext =
-        pantheonPluginContextMap.computeIfAbsent(node, n -> buildPluginContext(node));
-    pantheonPluginContext.addService(PicoCLIOptions.class, new PicoCLIOptionsImpl(commandLine));
+        pantheonPluginContextMap.computeIfAbsent(node, n -> buildPluginContext(node, commandLine));
 
     commandLine.parseArgs(node.getConfiguration().getExtraCLIOptions().toArray(new String[0]));
 
@@ -126,7 +127,7 @@ public void startNode(final PantheonNode node) {
 
     final RunnerBuilder runnerBuilder = new RunnerBuilder();
     if (node.getPermissioningConfiguration().isPresent()) {
-      PermissioningConfiguration permissioningConfiguration =
+      final PermissioningConfiguration permissioningConfiguration =
           node.getPermissioningConfiguration().get();
 
       runnerBuilder.permissioningConfiguration(permissioningConfiguration);
@@ -151,6 +152,7 @@ public void startNode(final PantheonNode node) {
             .webSocketConfiguration(node.webSocketConfiguration())
             .dataDir(node.homeDirectory())
             .metricsSystem(noOpMetricsSystem)
+            .clock(Clock.systemUTC())
             .metricsConfiguration(node.metricsConfiguration())
             .p2pEnabled(node.isP2pEnabled())
             .graphQLConfiguration(GraphQLConfiguration.createDefault())
@@ -167,7 +169,7 @@ public void startNode(final PantheonNode node) {
 
   @Override
   public void stopNode(final PantheonNode node) {
-    PantheonPluginContextImpl pluginContext = pantheonPluginContextMap.remove(node);
+    final PantheonPluginContextImpl pluginContext = pantheonPluginContextMap.remove(node);
     if (pluginContext != null) {
       pluginContext.stopPlugins();
     }

diff --git a/...main/java/tech/pegasys/pantheon/consensus/clique/BlockHeaderValidationRulesetFactory.java b/...main/java/tech/pegasys/pantheon/consensus/clique/BlockHeaderValidationRulesetFactory.java
@@ -28,6 +28,8 @@
 import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.TimestampBoundedByFutureParameter;
 import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.TimestampMoreRecentThanParent;
 
+import java.time.Clock;
+
 public class BlockHeaderValidationRulesetFactory {
 
   /**
@@ -38,16 +40,17 @@ public class BlockHeaderValidationRulesetFactory {
    *
    * @param secondsBetweenBlocks the minimum number of seconds which must elapse between blocks.
    * @param epochManager an object which determines if a given block is an epoch block.
+   * @param clock System clock
    * @return the header validator.
    */
   public static BlockHeaderValidator<CliqueContext> cliqueBlockHeaderValidator(
-      final long secondsBetweenBlocks, final EpochManager epochManager) {
+      final long secondsBetweenBlocks, final EpochManager epochManager, final Clock clock) {
 
     return new BlockHeaderValidator.Builder<CliqueContext>()
         .addRule(new AncestryValidationRule())
         .addRule(new GasUsageValidationRule())
         .addRule(new GasLimitRangeAndDeltaValidationRule(5000, 0x7fffffffffffffffL))
-        .addRule(new TimestampBoundedByFutureParameter(10))
+        .addRule(new TimestampBoundedByFutureParameter(10, clock))
         .addRule(new TimestampMoreRecentThanParent(secondsBetweenBlocks))
         .addRule(new ConstantFieldValidationRule<>("MixHash", BlockHeader::getMixHash, Hash.ZERO))
         .addRule(

diff --git a/...s/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueProtocolSchedule.java b/...s/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueProtocolSchedule.java
@@ -30,6 +30,7 @@
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSpecBuilder;
 
 import java.math.BigInteger;
+import java.time.Clock;
 
 /** Defines the protocol behaviours for a blockchain using Clique. */
 public class CliqueProtocolSchedule {
@@ -40,7 +41,8 @@ public static ProtocolSchedule<CliqueContext> create(
       final GenesisConfigOptions config,
       final KeyPair nodeKeys,
       final PrivacyParameters privacyParameters,
-      final boolean isRevertReasonEnabled) {
+      final boolean isRevertReasonEnabled,
+      final Clock clock) {
 
     final CliqueConfigOptions cliqueConfig = config.getCliqueConfigOptions();
 
@@ -52,28 +54,37 @@ public static ProtocolSchedule<CliqueContext> create(
             DEFAULT_CHAIN_ID,
             builder ->
                 applyCliqueSpecificModifications(
-                    epochManager, cliqueConfig.getBlockPeriodSeconds(), localNodeAddress, builder),
+                    epochManager,
+                    cliqueConfig.getBlockPeriodSeconds(),
+                    localNodeAddress,
+                    builder,
+                    clock),
             privacyParameters,
-            isRevertReasonEnabled)
+            isRevertReasonEnabled,
+            clock)
         .createProtocolSchedule();
   }
 
   public static ProtocolSchedule<CliqueContext> create(
       final GenesisConfigOptions config,
       final KeyPair nodeKeys,
-      final boolean isRevertReasonEnabled) {
-    return create(config, nodeKeys, PrivacyParameters.DEFAULT, isRevertReasonEnabled);
+      final boolean isRevertReasonEnabled,
+      final Clock clock) {
+    return create(config, nodeKeys, PrivacyParameters.DEFAULT, isRevertReasonEnabled, clock);
   }
 
   private static ProtocolSpecBuilder<CliqueContext> applyCliqueSpecificModifications(
       final EpochManager epochManager,
       final long secondsBetweenBlocks,
       final Address localNodeAddress,
-      final ProtocolSpecBuilder<Void> specBuilder) {
+      final ProtocolSpecBuilder<Void> specBuilder,
+      final Clock clock) {
     return specBuilder
         .changeConsensusContextType(
-            difficultyCalculator -> cliqueBlockHeaderValidator(secondsBetweenBlocks, epochManager),
-            difficultyCalculator -> cliqueBlockHeaderValidator(secondsBetweenBlocks, epochManager),
+            difficultyCalculator ->
+                cliqueBlockHeaderValidator(secondsBetweenBlocks, epochManager, clock),
+            difficultyCalculator ->
+                cliqueBlockHeaderValidator(secondsBetweenBlocks, epochManager, clock),
             MainnetBlockBodyValidator::new,
             MainnetBlockValidator::new,
             MainnetBlockImporter::new,

diff --git a/...ique/src/test/java/tech/pegasys/pantheon/consensus/clique/CliqueProtocolScheduleTest.java b/...ique/src/test/java/tech/pegasys/pantheon/consensus/clique/CliqueProtocolScheduleTest.java
@@ -20,6 +20,7 @@
 import tech.pegasys.pantheon.ethereum.core.Wei;
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule;
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSpec;
+import tech.pegasys.pantheon.testutil.TestClock;
 
 import org.junit.Test;
 
@@ -41,7 +42,7 @@ public void protocolSpecsAreCreatedAtBlockDefinedInJson() {
 
     final GenesisConfigOptions config = GenesisConfigFile.fromConfig(jsonInput).getConfigOptions();
     final ProtocolSchedule<CliqueContext> protocolSchedule =
-        CliqueProtocolSchedule.create(config, NODE_KEYS, false);
+        CliqueProtocolSchedule.create(config, NODE_KEYS, false, TestClock.fixed());
 
     final ProtocolSpec<CliqueContext> homesteadSpec = protocolSchedule.getByBlockNumber(1);
     final ProtocolSpec<CliqueContext> tangerineWhistleSpec = protocolSchedule.getByBlockNumber(2);
@@ -57,7 +58,7 @@ public void protocolSpecsAreCreatedAtBlockDefinedInJson() {
   public void parametersAlignWithMainnetWithAdjustments() {
     final ProtocolSpec<CliqueContext> homestead =
         CliqueProtocolSchedule.create(
-                GenesisConfigFile.DEFAULT.getConfigOptions(), NODE_KEYS, false)
+                GenesisConfigFile.DEFAULT.getConfigOptions(), NODE_KEYS, false, TestClock.fixed())
             .getByBlockNumber(0);
 
     assertThat(homestead.getName()).isEqualTo("Frontier");

diff --git a/...est/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockCreatorTest.java b/...est/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockCreatorTest.java
@@ -76,7 +76,10 @@ public class CliqueBlockCreatorTest {
   public void setup() {
     protocolSchedule =
         CliqueProtocolSchedule.create(
-            GenesisConfigFile.DEFAULT.getConfigOptions(), proposerKeyPair, false);
+            GenesisConfigFile.DEFAULT.getConfigOptions(),
+            proposerKeyPair,
+            false,
+            TestClock.fixed());
 
     final Address otherAddress = Util.publicKeyToAddress(otherKeyPair.getPublicKey());
     validatorList.add(otherAddress);

diff --git a/...st/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMinerExecutorTest.java b/...st/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMinerExecutorTest.java
@@ -90,7 +90,8 @@ public void extraDataCreatedOnEpochBlocksContainsValidators() {
         new CliqueMinerExecutor(
             cliqueProtocolContext,
             Executors.newSingleThreadExecutor(),
-            CliqueProtocolSchedule.create(GENESIS_CONFIG_OPTIONS, proposerKeyPair, false),
+            CliqueProtocolSchedule.create(
+                GENESIS_CONFIG_OPTIONS, proposerKeyPair, false, TestClock.fixed()),
             new PendingTransactions(
                 TransactionPoolConfiguration.DEFAULT_TX_RETENTION_HOURS,
                 1,
@@ -127,7 +128,8 @@ public void extraDataForNonEpochBlocksDoesNotContainValidaors() {
         new CliqueMinerExecutor(
             cliqueProtocolContext,
             Executors.newSingleThreadExecutor(),
-            CliqueProtocolSchedule.create(GENESIS_CONFIG_OPTIONS, proposerKeyPair, false),
+            CliqueProtocolSchedule.create(
+                GENESIS_CONFIG_OPTIONS, proposerKeyPair, false, TestClock.fixed()),
             new PendingTransactions(
                 TransactionPoolConfiguration.DEFAULT_TX_RETENTION_HOURS,
                 1,
@@ -164,7 +166,8 @@ public void shouldUseLatestVanityData() {
         new CliqueMinerExecutor(
             cliqueProtocolContext,
             Executors.newSingleThreadExecutor(),
-            CliqueProtocolSchedule.create(GENESIS_CONFIG_OPTIONS, proposerKeyPair, false),
+            CliqueProtocolSchedule.create(
+                GENESIS_CONFIG_OPTIONS, proposerKeyPair, false, TestClock.fixed()),
             new PendingTransactions(
                 TransactionPoolConfiguration.DEFAULT_TX_RETENTION_HOURS,
                 1,

diff --git a/...ntegration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextBuilder.java b/...ntegration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextBuilder.java
@@ -12,6 +12,7 @@
  */
 package tech.pegasys.pantheon.consensus.ibft.support;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.mockito.Mockito.mock;
 import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.createInMemoryBlockchain;
@@ -159,6 +160,8 @@ public TestContextBuilder useGossip(final boolean useGossip) {
   }
 
   public TestContext build() {
+    checkNotNull(clock);
+
     final NetworkLayout networkNodes =
         NetworkLayout.createNetworkLayout(validatorCount, indexOfFirstLocallyProposedBlock);
 
@@ -263,7 +266,7 @@ private static ControllerAndState createControllerAndFinalState(
     genesisConfigOptions.byzantiumBlock(0);
 
     final ProtocolSchedule<IbftContext> protocolSchedule =
-        IbftProtocolSchedule.create(genesisConfigOptions);
+        IbftProtocolSchedule.create(genesisConfigOptions, Clock.systemUTC());
 
     /////////////////////////////////////////////////////////////////////////////////////
     // From here down is BASICALLY taken from IbftPantheonController

diff --git a/...in/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockHeaderValidationRulesetFactory.java b/...in/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockHeaderValidationRulesetFactory.java
@@ -26,22 +26,25 @@
 import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.TimestampMoreRecentThanParent;
 import tech.pegasys.pantheon.util.uint.UInt256;
 
+import java.time.Clock;
+
 public class IbftBlockHeaderValidationRulesetFactory {
 
   /**
    * Produces a BlockHeaderValidator configured for assessing ibft block headers which are to form
    * part of the BlockChain (i.e. not proposed blocks, which do not contain commit seals)
    *
    * @param secondsBetweenBlocks the minimum number of seconds which must elapse between blocks.
+   * @param clock System clock
    * @return BlockHeaderValidator configured for assessing ibft block headers
    */
   public static BlockHeaderValidator<IbftContext> ibftBlockHeaderValidator(
-      final long secondsBetweenBlocks) {
+      final long secondsBetweenBlocks, final Clock clock) {
     return new BlockHeaderValidator.Builder<IbftContext>()
         .addRule(new AncestryValidationRule())
         .addRule(new GasUsageValidationRule())
         .addRule(new GasLimitRangeAndDeltaValidationRule(5000, 0x7fffffffffffffffL))
-        .addRule(new TimestampBoundedByFutureParameter(1))
+        .addRule(new TimestampBoundedByFutureParameter(1, clock))
         .addRule(new TimestampMoreRecentThanParent(secondsBetweenBlocks))
         .addRule(
             new ConstantFieldValidationRule<>(

diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolSchedule.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolSchedule.java
@@ -26,6 +26,7 @@
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSpecBuilder;
 
 import java.math.BigInteger;
+import java.time.Clock;
 
 /** Defines the protocol behaviours for a blockchain using IBFT. */
 public class IbftProtocolSchedule {
@@ -35,34 +36,37 @@ public class IbftProtocolSchedule {
   public static ProtocolSchedule<IbftContext> create(
       final GenesisConfigOptions config,
       final PrivacyParameters privacyParameters,
-      final boolean isRevertReasonEnabled) {
+      final boolean isRevertReasonEnabled,
+      final Clock clock) {
     final IbftConfigOptions ibftConfig = config.getIbftLegacyConfigOptions();
     final long blockPeriod = ibftConfig.getBlockPeriodSeconds();
 
     return new ProtocolScheduleBuilder<>(
             config,
             DEFAULT_CHAIN_ID,
-            builder -> applyIbftChanges(blockPeriod, builder),
+            builder -> applyIbftChanges(blockPeriod, builder, clock),
             privacyParameters,
-            isRevertReasonEnabled)
+            isRevertReasonEnabled,
+            clock)
         .createProtocolSchedule();
   }
 
   public static ProtocolSchedule<IbftContext> create(
-      final GenesisConfigOptions config, final boolean isRevertReasonEnabled) {
-    return create(config, PrivacyParameters.DEFAULT, isRevertReasonEnabled);
+      final GenesisConfigOptions config, final boolean isRevertReasonEnabled, final Clock clock) {
+    return create(config, PrivacyParameters.DEFAULT, isRevertReasonEnabled, clock);
   }
 
-  public static ProtocolSchedule<IbftContext> create(final GenesisConfigOptions config) {
-    return create(config, PrivacyParameters.DEFAULT, false);
+  public static ProtocolSchedule<IbftContext> create(
+      final GenesisConfigOptions config, final Clock clock) {
+    return create(config, PrivacyParameters.DEFAULT, false, clock);
   }
 
   private static ProtocolSpecBuilder<IbftContext> applyIbftChanges(
-      final long secondsBetweenBlocks, final ProtocolSpecBuilder<Void> builder) {
+      final long secondsBetweenBlocks, final ProtocolSpecBuilder<Void> builder, final Clock clock) {
     return builder
         .<IbftContext>changeConsensusContextType(
-            difficultyCalculator -> ibftBlockHeaderValidator(secondsBetweenBlocks),
-            difficultyCalculator -> ibftBlockHeaderValidator(secondsBetweenBlocks),
+            difficultyCalculator -> ibftBlockHeaderValidator(secondsBetweenBlocks, clock),
+            difficultyCalculator -> ibftBlockHeaderValidator(secondsBetweenBlocks, clock),
             MainnetBlockBodyValidator::new,
             MainnetBlockValidator::new,
             MainnetBlockImporter::new,