From a1e6adfcba817a8fc177582960611eb7e4ed4661 Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Wed, 31 Oct 2018 14:47:44 -0600 Subject: [PATCH] Replace custom Clock with java.time.Clock (#220) We create our own instance of Clock. However the java.time.Clock provides what is needed and already exists on the platform. --- .../blockcreation/CliqueBlockScheduler.java | 2 +- .../CliqueBlockSchedulerTest.java | 8 +++---- .../pantheon/consensus/ibft/BlockTimer.java | 4 ++-- .../consensus/ibft/BlockTimerTest.java | 14 ++++++------- .../blockcreation/AbstractBlockScheduler.java | 3 ++- .../blockcreation/DefaultBlockScheduler.java | 4 ++-- .../DefaultBlockSchedulerTest.java | 12 +++++------ .../EthHashMinerExecutorTest.java | 6 +++--- .../controller/CliquePantheonController.java | 4 ++-- .../controller/MainnetPantheonController.java | 4 ++-- .../pegasys/pantheon/util/time/Clock.java | 18 ---------------- .../pantheon/util/time/SystemClock.java | 21 ------------------- 12 files changed, 31 insertions(+), 69 deletions(-) delete mode 100644 util/src/main/java/tech/pegasys/pantheon/util/time/Clock.java delete mode 100644 util/src/main/java/tech/pegasys/pantheon/util/time/SystemClock.java diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockScheduler.java b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockScheduler.java index 9c0c68e707..db0507864d 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockScheduler.java +++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockScheduler.java @@ -17,8 +17,8 @@ import tech.pegasys.pantheon.consensus.common.ValidatorProvider; import tech.pegasys.pantheon.ethereum.blockcreation.DefaultBlockScheduler; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; -import tech.pegasys.pantheon.util.time.Clock; +import java.time.Clock; import java.util.Random; import com.google.common.annotations.VisibleForTesting; diff --git a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockSchedulerTest.java b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockSchedulerTest.java index e730d49343..825c8b5aff 100644 --- a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockSchedulerTest.java +++ b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockSchedulerTest.java @@ -26,8 +26,8 @@ import tech.pegasys.pantheon.ethereum.core.AddressHelpers; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.core.BlockHeaderTestFixture; import tech.pegasys.pantheon.ethereum.core.Util; -import tech.pegasys.pantheon.util.time.Clock; +import java.time.Clock; import java.util.List; import com.google.common.collect.Lists; @@ -61,7 +61,7 @@ public class CliqueBlockSchedulerTest { final Clock clock = mock(Clock.class); final long currentSecondsSinceEpoch = 10L; final long secondsBetweenBlocks = 5L; - when(clock.millisecondsSinceEpoch()).thenReturn(currentSecondsSinceEpoch * 1000); + when(clock.millis()).thenReturn(currentSecondsSinceEpoch * 1000); final CliqueBlockScheduler scheduler = new CliqueBlockScheduler(clock, voteTallyCache, localAddr, secondsBetweenBlocks); @@ -82,7 +82,7 @@ public class CliqueBlockSchedulerTest { final Clock clock = mock(Clock.class); final long currentSecondsSinceEpoch = 10L; final long secondsBetweenBlocks = 5L; - when(clock.millisecondsSinceEpoch()).thenReturn(currentSecondsSinceEpoch * 1000); + when(clock.millis()).thenReturn(currentSecondsSinceEpoch * 1000); final CliqueBlockScheduler scheduler = new CliqueBlockScheduler(clock, voteTallyCache, localAddr, secondsBetweenBlocks); @@ -103,7 +103,7 @@ public class CliqueBlockSchedulerTest { final Clock clock = mock(Clock.class); final long currentSecondsSinceEpoch = 10L; final long secondsBetweenBlocks = 5L; - when(clock.millisecondsSinceEpoch()).thenReturn(currentSecondsSinceEpoch * 1000); + when(clock.millis()).thenReturn(currentSecondsSinceEpoch * 1000); final CliqueBlockScheduler scheduler = new CliqueBlockScheduler(clock, voteTallyCache, localAddr, secondsBetweenBlocks); diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/BlockTimer.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/BlockTimer.java index ec311cc0ca..66cae85bee 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/BlockTimer.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/BlockTimer.java @@ -14,8 +14,8 @@ package tech.pegasys.pantheon.consensus.ibft; import tech.pegasys.pantheon.consensus.ibft.ibftevent.BlockTimerExpiry; import tech.pegasys.pantheon.ethereum.core.BlockHeader; -import tech.pegasys.pantheon.util.time.Clock; +import java.time.Clock; import java.util.Optional; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; @@ -74,7 +74,7 @@ public class BlockTimer { final ConsensusRoundIdentifier round, final BlockHeader chainHeadHeader) { cancelTimer(); - final long now = clock.millisecondsSinceEpoch(); + final long now = clock.millis(); // absolute time when the timer is supposed to expire final long expiryTime = chainHeadHeader.getTimestamp() * 1_000 + minimumTimeBetweenBlocksMillis; diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/BlockTimerTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/BlockTimerTest.java index 3bc68167a4..0c0d300bad 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/BlockTimerTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/BlockTimerTest.java @@ -27,8 +27,8 @@ import static org.mockito.Mockito.when; import tech.pegasys.pantheon.consensus.ibft.ibftevent.BlockTimerExpiry; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.core.BlockHeaderTestFixture; -import tech.pegasys.pantheon.util.time.Clock; +import java.time.Clock; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; @@ -76,7 +76,7 @@ public class BlockTimerTest { new BlockTimer( mockQueue, MINIMAL_TIME_BETWEEN_BLOCKS_MILLIS, mockExecutorService, mockClock); - when(mockClock.millisecondsSinceEpoch()).thenReturn(NOW_MILLIS); + when(mockClock.millis()).thenReturn(NOW_MILLIS); final BlockHeader header = new BlockHeaderTestFixture().timestamp(BLOCK_TIME_STAMP).buildHeader(); @@ -101,7 +101,7 @@ public class BlockTimerTest { final long BLOCK_TIME_STAMP = 300; final long EXPECTED_DELAY = 500; - when(mockClock.millisecondsSinceEpoch()).thenReturn(NOW_MILLIS); + when(mockClock.millis()).thenReturn(NOW_MILLIS); final BlockHeader header = new BlockHeaderTestFixture().timestamp(BLOCK_TIME_STAMP).buildHeader(); @@ -142,7 +142,7 @@ public class BlockTimerTest { new BlockTimer( mockQueue, MINIMAL_TIME_BETWEEN_BLOCKS_MILLIS, mockExecutorService, mockClock); - when(mockClock.millisecondsSinceEpoch()).thenReturn(NOW_MILLIS); + when(mockClock.millis()).thenReturn(NOW_MILLIS); final BlockHeader header = new BlockHeaderTestFixture().timestamp(BLOCK_TIME_STAMP).buildHeader(); @@ -170,7 +170,7 @@ public class BlockTimerTest { new BlockTimer( mockQueue, MINIMAL_TIME_BETWEEN_BLOCKS_MILLIS, mockExecutorService, mockClock); - when(mockClock.millisecondsSinceEpoch()).thenReturn(NOW_MILLIS); + when(mockClock.millis()).thenReturn(NOW_MILLIS); final BlockHeader header = new BlockHeaderTestFixture().timestamp(BLOCK_TIME_STAMP).buildHeader(); @@ -198,7 +198,7 @@ public class BlockTimerTest { new BlockTimer( mockQueue, MINIMAL_TIME_BETWEEN_BLOCKS_MILLIS, mockExecutorService, mockClock); - when(mockClock.millisecondsSinceEpoch()).thenReturn(NOW_MILLIS); + when(mockClock.millis()).thenReturn(NOW_MILLIS); final BlockHeader header = new BlockHeaderTestFixture().timestamp(BLOCK_TIME_STAMP).buildHeader(); @@ -225,7 +225,7 @@ public class BlockTimerTest { new BlockTimer( mockQueue, MINIMAL_TIME_BETWEEN_BLOCKS_MILLIS, mockExecutorService, mockClock); - when(mockClock.millisecondsSinceEpoch()).thenReturn(NOW_MILLIS); + when(mockClock.millis()).thenReturn(NOW_MILLIS); final BlockHeader header = new BlockHeaderTestFixture().timestamp(BLOCK_TIME_STAMP).buildHeader(); diff --git a/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/AbstractBlockScheduler.java b/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/AbstractBlockScheduler.java index 50a940484f..71026802d4 100644 --- a/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/AbstractBlockScheduler.java +++ b/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/AbstractBlockScheduler.java @@ -13,7 +13,8 @@ package tech.pegasys.pantheon.ethereum.blockcreation; import tech.pegasys.pantheon.ethereum.core.BlockHeader; -import tech.pegasys.pantheon.util.time.Clock; + +import java.time.Clock; public abstract class AbstractBlockScheduler { diff --git a/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/DefaultBlockScheduler.java b/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/DefaultBlockScheduler.java index 56a95f57e4..b454e73b3c 100644 --- a/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/DefaultBlockScheduler.java +++ b/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/DefaultBlockScheduler.java @@ -13,8 +13,8 @@ package tech.pegasys.pantheon.ethereum.blockcreation; import tech.pegasys.pantheon.ethereum.core.BlockHeader; -import tech.pegasys.pantheon.util.time.Clock; +import java.time.Clock; import java.util.concurrent.TimeUnit; import com.google.common.annotations.VisibleForTesting; @@ -36,7 +36,7 @@ public class DefaultBlockScheduler extends AbstractBlockScheduler { @Override @VisibleForTesting public BlockCreationTimeResult getNextTimestamp(final BlockHeader parentHeader) { - final long msSinceEpoch = clock.millisecondsSinceEpoch(); + final long msSinceEpoch = clock.millis(); final long now = TimeUnit.SECONDS.convert(msSinceEpoch, TimeUnit.MILLISECONDS); final long parentTimestamp = parentHeader.getTimestamp(); diff --git a/ethereum/blockcreation/src/test/java/tech/pegasys/pantheon/ethereum/blockcreation/DefaultBlockSchedulerTest.java b/ethereum/blockcreation/src/test/java/tech/pegasys/pantheon/ethereum/blockcreation/DefaultBlockSchedulerTest.java index 07bbf3b249..46684ce370 100644 --- a/ethereum/blockcreation/src/test/java/tech/pegasys/pantheon/ethereum/blockcreation/DefaultBlockSchedulerTest.java +++ b/ethereum/blockcreation/src/test/java/tech/pegasys/pantheon/ethereum/blockcreation/DefaultBlockSchedulerTest.java @@ -19,7 +19,8 @@ import static org.mockito.Mockito.when; import tech.pegasys.pantheon.ethereum.blockcreation.AbstractBlockScheduler.BlockCreationTimeResult; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.core.BlockHeaderTestFixture; -import tech.pegasys.pantheon.util.time.Clock; + +import java.time.Clock; import org.junit.Test; @@ -38,7 +39,7 @@ public class DefaultBlockSchedulerTest { // Determine the system time of parent block creation, which means child will occur on // the limit of clock drift. final long parentBlockSystemTimeCreation = parentTimeStamp - acceptableClockDrift + 1; - when(clock.millisecondsSinceEpoch()).thenReturn(parentBlockSystemTimeCreation * 1000); + when(clock.millis()).thenReturn(parentBlockSystemTimeCreation * 1000); final BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture(); final BlockHeader parentBlock = headerBuilder.timestamp(parentTimeStamp).buildHeader(); @@ -54,7 +55,7 @@ public class DefaultBlockSchedulerTest { final DefaultBlockScheduler scheduler = new DefaultBlockScheduler(interBlockSeconds, acceptableClockDrift, clock); - when(clock.millisecondsSinceEpoch()).thenReturn(parentTimeStamp * 1000); + when(clock.millis()).thenReturn(parentTimeStamp * 1000); final BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture(); final BlockHeader parentBlock = headerBuilder.timestamp(parentTimeStamp).buildHeader(); @@ -70,8 +71,7 @@ public class DefaultBlockSchedulerTest { new DefaultBlockScheduler(interBlockSeconds, acceptableClockDrift, clock); // Set the clock such that the parenttimestamp is on the limit of acceptability - when(clock.millisecondsSinceEpoch()) - .thenReturn((parentTimeStamp - acceptableClockDrift) * 1000); + when(clock.millis()).thenReturn((parentTimeStamp - acceptableClockDrift) * 1000); final BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture(); final BlockHeader parentBlock = headerBuilder.timestamp(parentTimeStamp).buildHeader(); @@ -88,7 +88,7 @@ public class DefaultBlockSchedulerTest { final DefaultBlockScheduler scheduler = new DefaultBlockScheduler(interBlockSeconds, acceptableClockDrift, clock); - when(clock.millisecondsSinceEpoch()).thenReturn(secondsSinceEpoch * 1000); + when(clock.millis()).thenReturn(secondsSinceEpoch * 1000); final BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture(); final BlockHeader parentBlock = headerBuilder.timestamp(parentTimeStamp).buildHeader(); diff --git a/ethereum/blockcreation/src/test/java/tech/pegasys/pantheon/ethereum/blockcreation/EthHashMinerExecutorTest.java b/ethereum/blockcreation/src/test/java/tech/pegasys/pantheon/ethereum/blockcreation/EthHashMinerExecutorTest.java index 5cf0a8fa0d..a1847f6531 100644 --- a/ethereum/blockcreation/src/test/java/tech/pegasys/pantheon/ethereum/blockcreation/EthHashMinerExecutorTest.java +++ b/ethereum/blockcreation/src/test/java/tech/pegasys/pantheon/ethereum/blockcreation/EthHashMinerExecutorTest.java @@ -18,8 +18,8 @@ import tech.pegasys.pantheon.ethereum.core.MiningParameters; import tech.pegasys.pantheon.ethereum.core.MiningParametersTestBuilder; import tech.pegasys.pantheon.ethereum.core.PendingTransactions; import tech.pegasys.pantheon.util.Subscribers; -import tech.pegasys.pantheon.util.time.SystemClock; +import java.time.Clock; import java.util.concurrent.Executors; import org.junit.Test; @@ -38,7 +38,7 @@ public class EthHashMinerExecutorTest { null, new PendingTransactions(1), miningParameters, - new DefaultBlockScheduler(1, 10, new SystemClock())); + new DefaultBlockScheduler(1, 10, Clock.systemUTC())); assertThatExceptionOfType(CoinbaseNotSetException.class) .isThrownBy(() -> executor.startAsyncMining(new Subscribers<>(), null)) @@ -56,7 +56,7 @@ public class EthHashMinerExecutorTest { null, new PendingTransactions(1), miningParameters, - new DefaultBlockScheduler(1, 10, new SystemClock())); + new DefaultBlockScheduler(1, 10, Clock.systemUTC())); assertThatExceptionOfType(IllegalArgumentException.class) .isThrownBy(() -> executor.setCoinbase(null)) diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/controller/CliquePantheonController.java b/pantheon/src/main/java/tech/pegasys/pantheon/controller/CliquePantheonController.java index 630a1968d2..8a3fba44e2 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/controller/CliquePantheonController.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/controller/CliquePantheonController.java @@ -50,11 +50,11 @@ import tech.pegasys.pantheon.ethereum.p2p.config.SubProtocolConfiguration; import tech.pegasys.pantheon.ethereum.worldstate.KeyValueStorageWorldStateStorage; import tech.pegasys.pantheon.services.kvstore.KeyValueStorage; import tech.pegasys.pantheon.services.kvstore.RocksDbKeyValueStorage; -import tech.pegasys.pantheon.util.time.SystemClock; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Clock; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; @@ -170,7 +170,7 @@ public class CliquePantheonController implements PantheonController { new DefaultBlockScheduler( MainnetBlockHeaderValidator.MINIMUM_SECONDS_SINCE_PARENT, MainnetBlockHeaderValidator.TIMESTAMP_TOLERANCE_S, - new SystemClock())); + Clock.systemUTC())); final EthHashMiningCoordinator miningCoordinator = new EthHashMiningCoordinator(protocolContext.getBlockchain(), executor, syncState); diff --git a/util/src/main/java/tech/pegasys/pantheon/util/time/Clock.java b/util/src/main/java/tech/pegasys/pantheon/util/time/Clock.java deleted file mode 100644 index 3b15d64b89..0000000000 --- a/util/src/main/java/tech/pegasys/pantheon/util/time/Clock.java +++ /dev/null @@ -1,18 +0,0 @@ -/* - * Copyright 2018 ConsenSys AG. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ -package tech.pegasys.pantheon.util.time; - -public interface Clock { - - long millisecondsSinceEpoch(); -} diff --git a/util/src/main/java/tech/pegasys/pantheon/util/time/SystemClock.java b/util/src/main/java/tech/pegasys/pantheon/util/time/SystemClock.java deleted file mode 100644 index ad90fa5c16..0000000000 --- a/util/src/main/java/tech/pegasys/pantheon/util/time/SystemClock.java +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Copyright 2018 ConsenSys AG. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ -package tech.pegasys.pantheon.util.time; - -public class SystemClock implements Clock { - - @Override - public long millisecondsSinceEpoch() { - return System.currentTimeMillis(); - } -}