From db7acb06cd1c338acc11d3f7e04893ae69a38943 Mon Sep 17 00:00:00 2001 From: tmohay <37158202+rain-on@users.noreply.github.com> Date: Wed, 10 Oct 2018 11:13:09 +1100 Subject: [PATCH] Rework Clique Block Scheduler (#6) Clique Block Scheduler has been reworked to prevent high rate blocks being created when the parent block's timestamp is behind the system clock. --- .../blockcreation/CliqueBlockScheduler.java | 31 +++++++++++-------- .../CliqueBlockSchedulerTest.java | 2 +- ...duler.java => AbstractBlockScheduler.java} | 4 +-- .../ethereum/blockcreation/BlockMiner.java | 4 +-- .../blockcreation/DefaultBlockScheduler.java | 14 ++++----- .../blockcreation/EthHashBlockMiner.java | 2 +- .../blockcreation/EthHashMinerExecutor.java | 4 +-- .../DefaultBlockSchedulerTest.java | 2 +- 8 files changed, 33 insertions(+), 30 deletions(-) rename ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/{BaseBlockScheduler.java => AbstractBlockScheduler.java} (91%) diff --git a/consensus/clique/src/main/java/net/consensys/pantheon/consensus/clique/blockcreation/CliqueBlockScheduler.java b/consensus/clique/src/main/java/net/consensys/pantheon/consensus/clique/blockcreation/CliqueBlockScheduler.java index fab3cdb60e..c1003ac13f 100755 --- a/consensus/clique/src/main/java/net/consensys/pantheon/consensus/clique/blockcreation/CliqueBlockScheduler.java +++ b/consensus/clique/src/main/java/net/consensys/pantheon/consensus/clique/blockcreation/CliqueBlockScheduler.java @@ -2,7 +2,7 @@ package net.consensys.pantheon.consensus.clique.blockcreation; import net.consensys.pantheon.consensus.clique.VoteTallyCache; import net.consensys.pantheon.consensus.common.ValidatorProvider; -import net.consensys.pantheon.ethereum.blockcreation.BaseBlockScheduler; +import net.consensys.pantheon.ethereum.blockcreation.DefaultBlockScheduler; import net.consensys.pantheon.ethereum.core.Address; import net.consensys.pantheon.ethereum.core.BlockHeader; import net.consensys.pantheon.util.time.Clock; @@ -11,7 +11,7 @@ import java.util.Random; import com.google.common.annotations.VisibleForTesting; -public class CliqueBlockScheduler extends BaseBlockScheduler { +public class CliqueBlockScheduler extends DefaultBlockScheduler { private final int OUT_OF_TURN_DELAY_MULTIPLIER_MILLIS = 500; @@ -24,7 +24,7 @@ public class CliqueBlockScheduler extends BaseBlockScheduler { final VoteTallyCache voteTallyCache, final Address localNodeAddress, final long secondsBetweenBlocks) { - super(clock); + super(secondsBetweenBlocks, 0L, clock); this.voteTallyCache = voteTallyCache; this.localNodeAddress = localNodeAddress; this.secondsBetweenBlocks = secondsBetweenBlocks; @@ -33,24 +33,29 @@ public class CliqueBlockScheduler extends BaseBlockScheduler { @Override @VisibleForTesting public BlockCreationTimeResult getNextTimestamp(final BlockHeader parentHeader) { - final long nextHeaderTimestamp = parentHeader.getTimestamp() + secondsBetweenBlocks; - long milliSecondsUntilNextBlock = (nextHeaderTimestamp * 1000) - clock.millisecondsSinceEpoch(); + final BlockCreationTimeResult result = super.getNextTimestamp(parentHeader); - final CliqueProposerSelector proposerSelector = new CliqueProposerSelector(voteTallyCache); - final Address nextSelector = proposerSelector.selectProposerForNextBlock(parentHeader); - if (!nextSelector.equals(localNodeAddress)) { - milliSecondsUntilNextBlock += - calculatorOutOfTurnDelay(voteTallyCache.getVoteTallyAtBlock(parentHeader)); - } + final long milliSecondsUntilNextBlock = + result.getMillisecondsUntilValid() + calculateTurnBasedDelay(parentHeader); return new BlockCreationTimeResult( - nextHeaderTimestamp, Math.max(0, milliSecondsUntilNextBlock)); + result.getTimestampForHeader(), Math.max(0, milliSecondsUntilNextBlock)); + } + + private int calculateTurnBasedDelay(final BlockHeader parentHeader) { + final CliqueProposerSelector proposerSelector = new CliqueProposerSelector(voteTallyCache); + final Address nextProposer = proposerSelector.selectProposerForNextBlock(parentHeader); + + if (nextProposer.equals(localNodeAddress)) { + return 0; + } + return calculatorOutOfTurnDelay(voteTallyCache.getVoteTallyAtBlock(parentHeader)); } private int calculatorOutOfTurnDelay(final ValidatorProvider validators) { int countSigners = validators.getCurrentValidators().size(); int maxDelay = ((countSigners / 2) + 1) * OUT_OF_TURN_DELAY_MULTIPLIER_MILLIS; Random r = new Random(); - return r.nextInt((maxDelay) + 1); + return r.nextInt(maxDelay + 1); } } diff --git a/consensus/clique/src/test/java/net/consensys/pantheon/consensus/clique/blockcreation/CliqueBlockSchedulerTest.java b/consensus/clique/src/test/java/net/consensys/pantheon/consensus/clique/blockcreation/CliqueBlockSchedulerTest.java index fd01248552..8479b164dc 100755 --- a/consensus/clique/src/test/java/net/consensys/pantheon/consensus/clique/blockcreation/CliqueBlockSchedulerTest.java +++ b/consensus/clique/src/test/java/net/consensys/pantheon/consensus/clique/blockcreation/CliqueBlockSchedulerTest.java @@ -8,7 +8,7 @@ import static org.mockito.Mockito.when; import net.consensys.pantheon.consensus.clique.VoteTallyCache; import net.consensys.pantheon.consensus.common.VoteTally; import net.consensys.pantheon.crypto.SECP256K1.KeyPair; -import net.consensys.pantheon.ethereum.blockcreation.BaseBlockScheduler.BlockCreationTimeResult; +import net.consensys.pantheon.ethereum.blockcreation.AbstractBlockScheduler.BlockCreationTimeResult; import net.consensys.pantheon.ethereum.core.Address; import net.consensys.pantheon.ethereum.core.AddressHelpers; import net.consensys.pantheon.ethereum.core.BlockHeader; diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/BaseBlockScheduler.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/AbstractBlockScheduler.java similarity index 91% rename from ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/BaseBlockScheduler.java rename to ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/AbstractBlockScheduler.java index 6394fa9ef1..8b6a304682 100755 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/BaseBlockScheduler.java +++ b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/AbstractBlockScheduler.java @@ -3,11 +3,11 @@ package net.consensys.pantheon.ethereum.blockcreation; import net.consensys.pantheon.ethereum.core.BlockHeader; import net.consensys.pantheon.util.time.Clock; -public abstract class BaseBlockScheduler { +public abstract class AbstractBlockScheduler { protected final Clock clock; - public BaseBlockScheduler(final Clock clock) { + public AbstractBlockScheduler(final Clock clock) { this.clock = clock; } diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/BlockMiner.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/BlockMiner.java index 36750b16b5..2fb128c005 100755 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/BlockMiner.java +++ b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/BlockMiner.java @@ -32,7 +32,7 @@ public class BlockMiner implements Runnable { private final ProtocolContext protocolContext; private final ProtocolSchedule protocolSchedule; private final Subscribers observers; - private final BaseBlockScheduler scheduler; + private final AbstractBlockScheduler scheduler; private final BlockHeader parentHeader; public BlockMiner( @@ -40,7 +40,7 @@ public class BlockMiner implements Runnable { final ProtocolSchedule protocolSchedule, final ProtocolContext protocolContext, final Subscribers observers, - final BaseBlockScheduler scheduler, + final AbstractBlockScheduler scheduler, final BlockHeader parentHeader) { this.blockCreator = blockCreator; this.protocolContext = protocolContext; diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/DefaultBlockScheduler.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/DefaultBlockScheduler.java index 382075e4d1..bca72917e6 100755 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/DefaultBlockScheduler.java +++ b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/DefaultBlockScheduler.java @@ -7,7 +7,7 @@ import java.util.concurrent.TimeUnit; import com.google.common.annotations.VisibleForTesting; -public class DefaultBlockScheduler extends BaseBlockScheduler { +public class DefaultBlockScheduler extends AbstractBlockScheduler { private final long acceptableClockDriftSeconds; private final long minimumSecondsSinceParent; @@ -25,16 +25,14 @@ public class DefaultBlockScheduler extends BaseBlockScheduler { @VisibleForTesting public BlockCreationTimeResult getNextTimestamp(final BlockHeader parentHeader) { final long msSinceEpoch = clock.millisecondsSinceEpoch(); - final long secondsSinceEpoch = TimeUnit.SECONDS.convert(msSinceEpoch, TimeUnit.MILLISECONDS); + final long now = TimeUnit.SECONDS.convert(msSinceEpoch, TimeUnit.MILLISECONDS); final long parentTimestamp = parentHeader.getTimestamp(); - final long nextHeaderTimestamp = - Long.max(parentTimestamp + minimumSecondsSinceParent, secondsSinceEpoch); + final long nextHeaderTimestamp = Long.max(parentTimestamp + minimumSecondsSinceParent, now); - final long millisecondsUntilHeaderTimeStampIsValid = - (nextHeaderTimestamp * 1000) - (msSinceEpoch + (acceptableClockDriftSeconds * 1000)); + final long earliestBlockTransmissionTime = nextHeaderTimestamp - acceptableClockDriftSeconds; + final long msUntilBlocKTransmission = (earliestBlockTransmissionTime * 1000) - msSinceEpoch; - return new BlockCreationTimeResult( - nextHeaderTimestamp, Math.max(0, millisecondsUntilHeaderTimeStampIsValid)); + return new BlockCreationTimeResult(nextHeaderTimestamp, Math.max(0, msUntilBlocKTransmission)); } } diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/EthHashBlockMiner.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/EthHashBlockMiner.java index 3bb2536a19..b83311f6c1 100755 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/EthHashBlockMiner.java +++ b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/EthHashBlockMiner.java @@ -27,7 +27,7 @@ public class EthHashBlockMiner extends BlockMiner { final ProtocolSchedule protocolSchedule, final ProtocolContext protocolContext, final Subscribers observers, - final BaseBlockScheduler scheduler, + final AbstractBlockScheduler scheduler, final BlockHeader parentHeader) { super(blockCreator, protocolSchedule, protocolContext, observers, scheduler, parentHeader); this.blockCreator = blockCreator; diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/EthHashMinerExecutor.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/EthHashMinerExecutor.java index 6f1125332f..7692a29b09 100755 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/EthHashMinerExecutor.java +++ b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/blockcreation/EthHashMinerExecutor.java @@ -25,7 +25,7 @@ public class EthHashMinerExecutor { private volatile BytesValue extraData; private volatile Optional
coinbase; private volatile Wei minTransactionGasPrice; - private final BaseBlockScheduler blockScheduler; + private final AbstractBlockScheduler blockScheduler; public EthHashMinerExecutor( final ProtocolContext protocolContext, @@ -33,7 +33,7 @@ public class EthHashMinerExecutor { final ProtocolSchedule protocolSchedule, final PendingTransactions pendingTransactions, final MiningParameters miningParams, - final BaseBlockScheduler blockScheduler) { + final AbstractBlockScheduler blockScheduler) { this.protocolContext = protocolContext; this.executorService = executorService; this.protocolSchedule = protocolSchedule; diff --git a/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/blockcreation/DefaultBlockSchedulerTest.java b/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/blockcreation/DefaultBlockSchedulerTest.java index cb08acdf7a..16618c1511 100755 --- a/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/blockcreation/DefaultBlockSchedulerTest.java +++ b/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/blockcreation/DefaultBlockSchedulerTest.java @@ -4,7 +4,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import net.consensys.pantheon.ethereum.blockcreation.BaseBlockScheduler.BlockCreationTimeResult; +import net.consensys.pantheon.ethereum.blockcreation.AbstractBlockScheduler.BlockCreationTimeResult; import net.consensys.pantheon.ethereum.core.BlockHeader; import net.consensys.pantheon.ethereum.core.BlockHeaderTestFixture; import net.consensys.pantheon.util.time.Clock;