diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueBlockHashing.java b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueBlockHashing.java index de342cfa2f..a7579cfbbf 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueBlockHashing.java +++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueBlockHashing.java @@ -50,7 +50,7 @@ public class CliqueBlockHashing { final BlockHeader header, final CliqueExtraData cliqueExtraData) { if (!cliqueExtraData.getProposerSeal().isPresent()) { throw new IllegalArgumentException( - "Supplied cliqueExtraData does not include a proposer " + "seal"); + "Supplied cliqueExtraData does not include a proposer seal."); } final Hash proposerHash = calculateDataHashForProposerSeal(header, cliqueExtraData); return Util.signatureToAddress(cliqueExtraData.getProposerSeal().get(), proposerHash); diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueProposerSelector.java b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueProposerSelector.java index 282edc5056..4d91fd14ad 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueProposerSelector.java +++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueProposerSelector.java @@ -14,13 +14,14 @@ package tech.pegasys.pantheon.consensus.clique.blockcreation; import static com.google.common.base.Preconditions.checkNotNull; +import tech.pegasys.pantheon.consensus.clique.CliqueBlockInterface; import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.VoteTally; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; -import java.util.ArrayList; -import java.util.List; +import java.util.NavigableSet; +import java.util.SortedSet; /** * Responsible for determining which member of the validator pool should create the next block. @@ -31,6 +32,7 @@ import java.util.List; public class CliqueProposerSelector { private final VoteTallyCache voteTallyCache; + private final CliqueBlockInterface blockInterface = new CliqueBlockInterface(); public CliqueProposerSelector(final VoteTallyCache voteTallyCache) { checkNotNull(voteTallyCache); @@ -46,11 +48,26 @@ public class CliqueProposerSelector { public Address selectProposerForNextBlock(final BlockHeader parentHeader) { final VoteTally parentVoteTally = voteTallyCache.getVoteTallyAtBlock(parentHeader); - final List
validatorSet = new ArrayList<>(parentVoteTally.getCurrentValidators()); + final NavigableSet validatorSet = parentVoteTally.getCurrentValidators(); - final long nextBlockNumber = parentHeader.getNumber() + 1L; - final int indexIntoValidators = (int) (nextBlockNumber % validatorSet.size()); + Address prevBlockProposer = validatorSet.first(); + if (parentHeader.getNumber() != BlockHeader.GENESIS_BLOCK_NUMBER) { + prevBlockProposer = blockInterface.getProposerOfBlock(parentHeader); + } - return validatorSet.get(indexIntoValidators); + return selectNextProposer(prevBlockProposer, validatorSet); + } + + private Address selectNextProposer( + final Address prevBlockProposer, final NavigableSet validatorSet) { + final SortedSet latterValidators = validatorSet.tailSet(prevBlockProposer, false); + if (latterValidators.isEmpty()) { + // i.e. prevBlockProposer was at the end of the validator list, so the right validator for + // the start of this round is the first. + return validatorSet.first(); + } else { + // Else, use the first validator after the dropped entry. + return latterValidators.first(); + } } } diff --git a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/CliqueDifficultyCalculatorTest.java b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/CliqueDifficultyCalculatorTest.java index ad11565b1e..79dd394e5b 100644 --- a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/CliqueDifficultyCalculatorTest.java +++ b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/CliqueDifficultyCalculatorTest.java @@ -16,6 +16,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static tech.pegasys.pantheon.consensus.clique.TestHelpers.createCliqueSignedBlockHeader; import tech.pegasys.pantheon.consensus.common.VoteProposer; import tech.pegasys.pantheon.consensus.common.VoteTally; @@ -57,25 +58,30 @@ public class CliqueDifficultyCalculatorTest { final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, voteProposer, null); cliqueProtocolContext = new ProtocolContext<>(null, null, cliqueContext); blockHeaderBuilder = new BlockHeaderTestFixture(); + blockHeaderBuilder.number(1); } @Test - public void inTurnValidatorProducesDifficultyOfTwo() { + public void outTurnValidatorProducesDifficultyOfOne() { + // The proposer created the last block, so the next block must be a difficulty of 1. final CliqueDifficultyCalculator calculator = new CliqueDifficultyCalculator(localAddr); - final BlockHeader parentHeader = blockHeaderBuilder.number(1).buildHeader(); + BlockHeader parentHeader = + createCliqueSignedBlockHeader(blockHeaderBuilder, proposerKeyPair, validatorList); assertThat(calculator.nextDifficulty(0, parentHeader, cliqueProtocolContext)) - .isEqualTo(BigInteger.valueOf(2)); + .isEqualTo(BigInteger.valueOf(1)); } @Test - public void outTurnValidatorProducesDifficultyOfOne() { - final CliqueDifficultyCalculator calculator = new CliqueDifficultyCalculator(localAddr); + public void inTurnValidatorCreatesDifficultyOfTwo() { + final CliqueDifficultyCalculator calculator = + new CliqueDifficultyCalculator(validatorList.get(1)); // i.e. not the proposer. - final BlockHeader parentHeader = blockHeaderBuilder.number(2).buildHeader(); + BlockHeader parentHeader = + createCliqueSignedBlockHeader(blockHeaderBuilder, proposerKeyPair, validatorList); assertThat(calculator.nextDifficulty(0, parentHeader, cliqueProtocolContext)) - .isEqualTo(BigInteger.valueOf(1)); + .isEqualTo(BigInteger.valueOf(2)); } } 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 825c8b5aff..28b66e9f6d 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 @@ -16,6 +16,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static tech.pegasys.pantheon.consensus.clique.TestHelpers.createCliqueSignedBlockHeader; import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.VoteTally; @@ -37,7 +38,8 @@ import org.junit.Test; public class CliqueBlockSchedulerTest { private final KeyPair proposerKeyPair = KeyPair.generate(); - private Address localAddr; + private Address localAddr = Util.publicKeyToAddress(proposerKeyPair.getPublicKey()); + private Address otherAddr = AddressHelpers.calculateAddressWithRespectTo(localAddr, 1); private final List validatorList = Lists.newArrayList(); private VoteTallyCache voteTallyCache; @@ -45,10 +47,9 @@ public class CliqueBlockSchedulerTest { @Before public void setup() { - localAddr = Util.publicKeyToAddress(proposerKeyPair.getPublicKey()); validatorList.add(localAddr); - validatorList.add(AddressHelpers.calculateAddressWithRespectTo(localAddr, 1)); + validatorList.add(otherAddr); voteTallyCache = mock(VoteTallyCache.class); when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList)); @@ -63,12 +64,11 @@ public class CliqueBlockSchedulerTest { final long secondsBetweenBlocks = 5L; when(clock.millis()).thenReturn(currentSecondsSinceEpoch * 1000); final CliqueBlockScheduler scheduler = - new CliqueBlockScheduler(clock, voteTallyCache, localAddr, secondsBetweenBlocks); + new CliqueBlockScheduler(clock, voteTallyCache, otherAddr, secondsBetweenBlocks); - // There are 2 validators, therefore block 2 will put localAddr as the in-turn voter, therefore - // parent block should be number 1. + blockHeaderBuilder.number(1).timestamp(currentSecondsSinceEpoch); final BlockHeader parentHeader = - blockHeaderBuilder.number(1).timestamp(currentSecondsSinceEpoch).buildHeader(); + createCliqueSignedBlockHeader(blockHeaderBuilder, proposerKeyPair, validatorList); final BlockCreationTimeResult result = scheduler.getNextTimestamp(parentHeader); @@ -86,10 +86,9 @@ public class CliqueBlockSchedulerTest { final CliqueBlockScheduler scheduler = new CliqueBlockScheduler(clock, voteTallyCache, localAddr, secondsBetweenBlocks); - // There are 2 validators, therefore block 3 will put localAddr as the out-turn voter, therefore - // parent block should be number 2. + blockHeaderBuilder.number(2).timestamp(currentSecondsSinceEpoch); final BlockHeader parentHeader = - blockHeaderBuilder.number(2).timestamp(currentSecondsSinceEpoch).buildHeader(); + createCliqueSignedBlockHeader(blockHeaderBuilder, proposerKeyPair, validatorList); final BlockCreationTimeResult result = scheduler.getNextTimestamp(parentHeader); @@ -105,16 +104,13 @@ public class CliqueBlockSchedulerTest { final long secondsBetweenBlocks = 5L; when(clock.millis()).thenReturn(currentSecondsSinceEpoch * 1000); final CliqueBlockScheduler scheduler = - new CliqueBlockScheduler(clock, voteTallyCache, localAddr, secondsBetweenBlocks); + new CliqueBlockScheduler(clock, voteTallyCache, otherAddr, secondsBetweenBlocks); // There are 2 validators, therefore block 2 will put localAddr as the in-turn voter, therefore // parent block should be number 1. + blockHeaderBuilder.number(1).timestamp(currentSecondsSinceEpoch - secondsBetweenBlocks); final BlockHeader parentHeader = - blockHeaderBuilder - .number(1) - .timestamp(currentSecondsSinceEpoch - secondsBetweenBlocks) - .buildHeader(); - + createCliqueSignedBlockHeader(blockHeaderBuilder, proposerKeyPair, validatorList); final BlockCreationTimeResult result = scheduler.getNextTimestamp(parentHeader); assertThat(result.getTimestampForHeader()).isEqualTo(currentSecondsSinceEpoch); diff --git a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueProposerSelectorTest.java b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueProposerSelectorTest.java index 7427881088..7779831bb0 100644 --- a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueProposerSelectorTest.java +++ b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueProposerSelectorTest.java @@ -17,46 +17,87 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import tech.pegasys.pantheon.consensus.clique.TestHelpers; import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.ethereum.core.Address; 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 java.util.Arrays; import java.util.List; +import com.google.common.collect.Lists; import org.junit.Before; import org.junit.Test; public class CliqueProposerSelectorTest { + private final KeyPair proposerKey = KeyPair.generate(); + private final Address proposerAddress = Util.publicKeyToAddress(proposerKey.getPublicKey()); + private final List validatorList = - Arrays.asList( - AddressHelpers.ofValue(1), - AddressHelpers.ofValue(2), - AddressHelpers.ofValue(3), - AddressHelpers.ofValue(4)); + Lists.newArrayList( + AddressHelpers.calculateAddressWithRespectTo(proposerAddress, -1), + proposerAddress, + AddressHelpers.calculateAddressWithRespectTo(proposerAddress, 1), + AddressHelpers.calculateAddressWithRespectTo(proposerAddress, 2)); private final VoteTally voteTally = new VoteTally(validatorList); private VoteTallyCache voteTallyCache; + private final BlockHeaderTestFixture headerBuilderFixture = new BlockHeaderTestFixture(); @Before public void setup() { voteTallyCache = mock(VoteTallyCache.class); when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(voteTally); + + headerBuilderFixture.number(2); } @Test - public void proposerForABlockIsBasedOnModBlockNumber() { + public void firstBlockAfterGenesisIsTheSecondValidator() { final BlockHeaderTestFixture headerBuilderFixture = new BlockHeaderTestFixture(); + final CliqueProposerSelector selector = new CliqueProposerSelector(voteTallyCache); + headerBuilderFixture.number(0); + + assertThat(selector.selectProposerForNextBlock(headerBuilderFixture.buildHeader())) + .isEqualTo(validatorList.get(1)); + } + + @Test + public void selectsNextProposerInValidatorSet() { + final BlockHeader parentHeader = + TestHelpers.createCliqueSignedBlockHeader(headerBuilderFixture, proposerKey, validatorList); + final CliqueProposerSelector selector = new CliqueProposerSelector(voteTallyCache); + + // Proposer is at index 1, so the next proposer is at index 2 + assertThat(selector.selectProposerForNextBlock(parentHeader)).isEqualTo(validatorList.get(2)); + } + + @Test + public void selectsNextIndexWhenProposerIsNotInValidatorsForBlock() { + final BlockHeader parentHeader = + TestHelpers.createCliqueSignedBlockHeader(headerBuilderFixture, proposerKey, validatorList); + final CliqueProposerSelector selector = new CliqueProposerSelector(voteTallyCache); + + validatorList.remove(proposerAddress); + + // As the proposer was removed (index 1), the next proposer should also be index 1 + assertThat(selector.selectProposerForNextBlock(parentHeader)).isEqualTo(validatorList.get(1)); + } + + @Test + public void singleValidatorFindsItselfAsNextProposer() { + final List localValidators = Lists.newArrayList(proposerAddress); + final VoteTally localVoteTally = new VoteTally(localValidators); + when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(localVoteTally); + + final BlockHeader parentHeader = + TestHelpers.createCliqueSignedBlockHeader(headerBuilderFixture, proposerKey, validatorList); + final CliqueProposerSelector selector = new CliqueProposerSelector(voteTallyCache); - for (int prevBlockNumber = 0; prevBlockNumber < 10; prevBlockNumber++) { - headerBuilderFixture.number(prevBlockNumber); - final CliqueProposerSelector selector = new CliqueProposerSelector(voteTallyCache); - final Address nextProposer = - selector.selectProposerForNextBlock(headerBuilderFixture.buildHeader()); - assertThat(nextProposer) - .isEqualTo(validatorList.get((prevBlockNumber + 1) % validatorList.size())); - } + assertThat(selector.selectProposerForNextBlock(parentHeader)).isEqualTo(proposerAddress); } } diff --git a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/headervalidationrules/CliqueDifficultyValidationRuleTest.java b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/headervalidationrules/CliqueDifficultyValidationRuleTest.java index 4bbdd40cd1..555fb9f746 100644 --- a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/headervalidationrules/CliqueDifficultyValidationRuleTest.java +++ b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/headervalidationrules/CliqueDifficultyValidationRuleTest.java @@ -17,23 +17,17 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import tech.pegasys.pantheon.consensus.clique.CliqueBlockHashing; import tech.pegasys.pantheon.consensus.clique.CliqueContext; -import tech.pegasys.pantheon.consensus.clique.CliqueExtraData; +import tech.pegasys.pantheon.consensus.clique.TestHelpers; import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.VoteProposer; import tech.pegasys.pantheon.consensus.common.VoteTally; -import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; -import tech.pegasys.pantheon.crypto.SECP256K1.Signature; import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.core.Address; -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.Hash; import tech.pegasys.pantheon.ethereum.core.Util; -import tech.pegasys.pantheon.util.bytes.BytesValue; import tech.pegasys.pantheon.util.uint.UInt256; import java.util.List; @@ -45,15 +39,15 @@ import org.junit.Test; public class CliqueDifficultyValidationRuleTest { private final KeyPair proposerKeyPair = KeyPair.generate(); + private final KeyPair otherKeyPair = KeyPair.generate(); private final List validatorList = Lists.newArrayList(); private ProtocolContext