Revert "Repair Clique Proposer Selection" #339 - Breaks Görli testnet (#343)

Reverts #339

Görli's testnet regularly has proposers miss their turn and their
genesis block actually had a bad validator specified (that was voted out
as soon as they could) So many blocks do not have the proper "proposer"
as their block generator. As a consequence the
CliqueDifficultyValidationRule fails regularly, in fact it fails at
block 3, so this should be easy to re-create locally.

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);

consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueProposerSelector.java

@@ -14,14 +14,13 @@ 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.NavigableSet;
+import java.util.ArrayList;
-import java.util.SortedSet;
+import java.util.List;
 
 /**
  * Responsible for determining which member of the validator pool should create the next block.
@@ -32,7 +31,6 @@ import java.util.SortedSet;
 public class CliqueProposerSelector {
 
   private final VoteTallyCache voteTallyCache;
-  private final CliqueBlockInterface blockInterface = new CliqueBlockInterface();
 
   public CliqueProposerSelector(final VoteTallyCache voteTallyCache) {
     checkNotNull(voteTallyCache);
@@ -48,26 +46,11 @@ public class CliqueProposerSelector {
   public Address selectProposerForNextBlock(final BlockHeader parentHeader) {
 
     final VoteTally parentVoteTally = voteTallyCache.getVoteTallyAtBlock(parentHeader);
-    final NavigableSet<Address> validatorSet = parentVoteTally.getCurrentValidators();
+    final List<Address> validatorSet = new ArrayList<>(parentVoteTally.getCurrentValidators());
 
-    Address prevBlockProposer = validatorSet.first();
+    final long nextBlockNumber = parentHeader.getNumber() + 1L;
-    if (parentHeader.getNumber() != BlockHeader.GENESIS_BLOCK_NUMBER) {
+    final int indexIntoValidators = (int) (nextBlockNumber % validatorSet.size());
-      prevBlockProposer = blockInterface.getProposerOfBlock(parentHeader);
-    }
 
-    return selectNextProposer(prevBlockProposer, validatorSet);
+    return validatorSet.get(indexIntoValidators);
-  }
-
-  private Address selectNextProposer(
-      final Address prevBlockProposer, final NavigableSet<Address> validatorSet) {
-    final SortedSet<Address> 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();
-    }
   }
 }

consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/CliqueDifficultyCalculatorTest.java

@@ -16,7 +16,6 @@ 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;
@@ -58,30 +57,25 @@ 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 outTurnValidatorProducesDifficultyOfOne() {
+  public void inTurnValidatorProducesDifficultyOfTwo() {
-    // The proposer created the last block, so the next block must be a difficulty of 1.
     final CliqueDifficultyCalculator calculator = new CliqueDifficultyCalculator(localAddr);
 
-    BlockHeader parentHeader =
+    final BlockHeader parentHeader = blockHeaderBuilder.number(1).buildHeader();
-        createCliqueSignedBlockHeader(blockHeaderBuilder, proposerKeyPair, validatorList);
 
     assertThat(calculator.nextDifficulty(0, parentHeader, cliqueProtocolContext))
-        .isEqualTo(BigInteger.valueOf(1));
+        .isEqualTo(BigInteger.valueOf(2));
   }
 
   @Test
-  public void inTurnValidatorCreatesDifficultyOfTwo() {
+  public void outTurnValidatorProducesDifficultyOfOne() {
-    final CliqueDifficultyCalculator calculator =
+    final CliqueDifficultyCalculator calculator = new CliqueDifficultyCalculator(localAddr);
-        new CliqueDifficultyCalculator(validatorList.get(1)); // i.e. not the proposer.
 
-    BlockHeader parentHeader =
+    final BlockHeader parentHeader = blockHeaderBuilder.number(2).buildHeader();
-        createCliqueSignedBlockHeader(blockHeaderBuilder, proposerKeyPair, validatorList);
 
     assertThat(calculator.nextDifficulty(0, parentHeader, cliqueProtocolContext))
-        .isEqualTo(BigInteger.valueOf(2));
+        .isEqualTo(BigInteger.valueOf(1));
   }
 }

consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockSchedulerTest.java

@@ -16,7 +16,6 @@ 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;
@@ -38,8 +37,7 @@ import org.junit.Test;
 public class CliqueBlockSchedulerTest {
 
   private final KeyPair proposerKeyPair = KeyPair.generate();
-  private Address localAddr = Util.publicKeyToAddress(proposerKeyPair.getPublicKey());
+  private Address localAddr;
-  private Address otherAddr = AddressHelpers.calculateAddressWithRespectTo(localAddr, 1);
 
   private final List<Address> validatorList = Lists.newArrayList();
   private VoteTallyCache voteTallyCache;
@@ -47,9 +45,10 @@ public class CliqueBlockSchedulerTest {
 
   @Before
   public void setup() {
+    localAddr = Util.publicKeyToAddress(proposerKeyPair.getPublicKey());
 
     validatorList.add(localAddr);
-    validatorList.add(otherAddr);
+    validatorList.add(AddressHelpers.calculateAddressWithRespectTo(localAddr, 1));
 
     voteTallyCache = mock(VoteTallyCache.class);
     when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList));
@@ -64,11 +63,12 @@ public class CliqueBlockSchedulerTest {
     final long secondsBetweenBlocks = 5L;
     when(clock.millis()).thenReturn(currentSecondsSinceEpoch * 1000);
     final CliqueBlockScheduler scheduler =
-        new CliqueBlockScheduler(clock, voteTallyCache, otherAddr, secondsBetweenBlocks);
+        new CliqueBlockScheduler(clock, voteTallyCache, localAddr, secondsBetweenBlocks);
 
-    blockHeaderBuilder.number(1).timestamp(currentSecondsSinceEpoch);
+    // There are 2 validators, therefore block 2 will put localAddr as the in-turn voter, therefore
+    // parent block should be number 1.
     final BlockHeader parentHeader =
-        createCliqueSignedBlockHeader(blockHeaderBuilder, proposerKeyPair, validatorList);
+        blockHeaderBuilder.number(1).timestamp(currentSecondsSinceEpoch).buildHeader();
 
     final BlockCreationTimeResult result = scheduler.getNextTimestamp(parentHeader);
 
@@ -86,9 +86,10 @@ public class CliqueBlockSchedulerTest {
     final CliqueBlockScheduler scheduler =
         new CliqueBlockScheduler(clock, voteTallyCache, localAddr, secondsBetweenBlocks);
 
-    blockHeaderBuilder.number(2).timestamp(currentSecondsSinceEpoch);
+    // There are 2 validators, therefore block 3 will put localAddr as the out-turn voter, therefore
+    // parent block should be number 2.
     final BlockHeader parentHeader =
-        createCliqueSignedBlockHeader(blockHeaderBuilder, proposerKeyPair, validatorList);
+        blockHeaderBuilder.number(2).timestamp(currentSecondsSinceEpoch).buildHeader();
 
     final BlockCreationTimeResult result = scheduler.getNextTimestamp(parentHeader);
 
@@ -104,13 +105,16 @@ public class CliqueBlockSchedulerTest {
     final long secondsBetweenBlocks = 5L;
     when(clock.millis()).thenReturn(currentSecondsSinceEpoch * 1000);
     final CliqueBlockScheduler scheduler =
-        new CliqueBlockScheduler(clock, voteTallyCache, otherAddr, secondsBetweenBlocks);
+        new CliqueBlockScheduler(clock, voteTallyCache, localAddr, 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 =
-        createCliqueSignedBlockHeader(blockHeaderBuilder, proposerKeyPair, validatorList);
+        blockHeaderBuilder
+            .number(1)
+            .timestamp(currentSecondsSinceEpoch - secondsBetweenBlocks)
+            .buildHeader();
+
     final BlockCreationTimeResult result = scheduler.getNextTimestamp(parentHeader);
 
     assertThat(result.getTimestampForHeader()).isEqualTo(currentSecondsSinceEpoch);

consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueProposerSelectorTest.java

@@ -17,87 +17,46 @@ 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<Address> validatorList =
-      Lists.newArrayList(
+      Arrays.asList(
-          AddressHelpers.calculateAddressWithRespectTo(proposerAddress, -1),
+          AddressHelpers.ofValue(1),
-          proposerAddress,
+          AddressHelpers.ofValue(2),
-          AddressHelpers.calculateAddressWithRespectTo(proposerAddress, 1),
+          AddressHelpers.ofValue(3),
-          AddressHelpers.calculateAddressWithRespectTo(proposerAddress, 2));
+          AddressHelpers.ofValue(4));
   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 firstBlockAfterGenesisIsTheSecondValidator() {
+  public void proposerForABlockIsBasedOnModBlockNumber() {
     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<Address> 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);
 
-    assertThat(selector.selectProposerForNextBlock(parentHeader)).isEqualTo(proposerAddress);
+    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()));
+    }
   }
 }

consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/headervalidationrules/CliqueDifficultyValidationRuleTest.java

@@ -17,17 +17,23 @@ 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.TestHelpers;
+import tech.pegasys.pantheon.consensus.clique.CliqueExtraData;
 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;
@@ -39,15 +45,15 @@ import org.junit.Test;
 public class CliqueDifficultyValidationRuleTest {
 
   private final KeyPair proposerKeyPair = KeyPair.generate();
-  private final KeyPair otherKeyPair = KeyPair.generate();
   private final List<Address> validatorList = Lists.newArrayList();
   private ProtocolContext<CliqueContext> cliqueProtocolContext;
   private BlockHeaderTestFixture blockHeaderBuilder;
 
   @Before
   public void setup() {
-    validatorList.add(Util.publicKeyToAddress(proposerKeyPair.getPublicKey()));
+    final Address localAddress = Util.publicKeyToAddress(proposerKeyPair.getPublicKey());
-    validatorList.add(Util.publicKeyToAddress(otherKeyPair.getPublicKey()));
+    validatorList.add(localAddress);
+    validatorList.add(AddressHelpers.calculateAddressWithRespectTo(localAddress, 1));
 
     final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class);
     when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList));
@@ -56,7 +62,29 @@ public class CliqueDifficultyValidationRuleTest {
     final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, voteProposer, null);
     cliqueProtocolContext = new ProtocolContext<>(null, null, cliqueContext);
     blockHeaderBuilder = new BlockHeaderTestFixture();
-    blockHeaderBuilder.number(2);
+  }
+
+  private BlockHeader createCliqueSignedBlock(final BlockHeaderTestFixture blockHeaderBuilder) {
+
+    final CliqueExtraData unsignedExtraData =
+        new CliqueExtraData(BytesValue.wrap(new byte[32]), null, validatorList);
+    blockHeaderBuilder.extraData(unsignedExtraData.encode());
+
+    final Hash signingHash =
+        CliqueBlockHashing.calculateDataHashForProposerSeal(
+            blockHeaderBuilder.buildHeader(), unsignedExtraData);
+
+    final Signature proposerSignature = SECP256K1.sign(signingHash, proposerKeyPair);
+
+    final CliqueExtraData signedExtraData =
+        new CliqueExtraData(
+            unsignedExtraData.getVanityData(),
+            proposerSignature,
+            unsignedExtraData.getValidators());
+
+    blockHeaderBuilder.extraData(signedExtraData.encode());
+
+    return blockHeaderBuilder.buildHeader();
   }
 
   @Test
@@ -64,13 +92,11 @@ public class CliqueDifficultyValidationRuleTest {
     final long IN_TURN_BLOCK_NUMBER = validatorList.size(); // i.e. proposer is 'in turn'
     final UInt256 REPORTED_DIFFICULTY = UInt256.of(2);
 
-    final BlockHeader parentHeader =
+    blockHeaderBuilder.number(IN_TURN_BLOCK_NUMBER - 1L);
-        TestHelpers.createCliqueSignedBlockHeader(blockHeaderBuilder, otherKeyPair, validatorList);
+    final BlockHeader parentHeader = createCliqueSignedBlock(blockHeaderBuilder);
 
     blockHeaderBuilder.number(IN_TURN_BLOCK_NUMBER).difficulty(REPORTED_DIFFICULTY);
-    final BlockHeader newBlock =
+    final BlockHeader newBlock = createCliqueSignedBlock(blockHeaderBuilder);
-        TestHelpers.createCliqueSignedBlockHeader(
-            blockHeaderBuilder, proposerKeyPair, validatorList);
 
     final CliqueDifficultyValidationRule diffValidationRule = new CliqueDifficultyValidationRule();
     assertThat(diffValidationRule.validate(newBlock, parentHeader, cliqueProtocolContext)).isTrue();
@@ -81,14 +107,11 @@ public class CliqueDifficultyValidationRuleTest {
     final long OUT_OF_TURN_BLOCK_NUMBER = validatorList.size() - 1L;
     final UInt256 REPORTED_DIFFICULTY = UInt256.of(1);
 
-    final BlockHeader parentHeader =
+    blockHeaderBuilder.number(OUT_OF_TURN_BLOCK_NUMBER - 1L);
-        TestHelpers.createCliqueSignedBlockHeader(
+    final BlockHeader parentHeader = createCliqueSignedBlock(blockHeaderBuilder);
-            blockHeaderBuilder, proposerKeyPair, validatorList);
 
     blockHeaderBuilder.number(OUT_OF_TURN_BLOCK_NUMBER).difficulty(REPORTED_DIFFICULTY);
-    final BlockHeader newBlock =
+    final BlockHeader newBlock = createCliqueSignedBlock(blockHeaderBuilder);
-        TestHelpers.createCliqueSignedBlockHeader(
-            blockHeaderBuilder, proposerKeyPair, validatorList);
 
     final CliqueDifficultyValidationRule diffValidationRule = new CliqueDifficultyValidationRule();
     assertThat(diffValidationRule.validate(newBlock, parentHeader, cliqueProtocolContext)).isTrue();
@@ -96,16 +119,14 @@ public class CliqueDifficultyValidationRuleTest {
 
   @Test
   public void isFalseIfOutTurnValidatorSuppliesDifficultyOfTwo() {
+    final long OUT_OF_TURN_BLOCK_NUMBER = validatorList.size() - 1L;
     final UInt256 REPORTED_DIFFICULTY = UInt256.of(2);
 
-    final BlockHeader parentHeader =
+    blockHeaderBuilder.number(OUT_OF_TURN_BLOCK_NUMBER - 1L);
-        TestHelpers.createCliqueSignedBlockHeader(
+    final BlockHeader parentHeader = createCliqueSignedBlock(blockHeaderBuilder);
-            blockHeaderBuilder, proposerKeyPair, validatorList);
 
-    blockHeaderBuilder.difficulty(REPORTED_DIFFICULTY);
+    blockHeaderBuilder.number(OUT_OF_TURN_BLOCK_NUMBER).difficulty(REPORTED_DIFFICULTY);
-    final BlockHeader newBlock =
+    final BlockHeader newBlock = createCliqueSignedBlock(blockHeaderBuilder);
-        TestHelpers.createCliqueSignedBlockHeader(
-            blockHeaderBuilder, proposerKeyPair, validatorList);
 
     final CliqueDifficultyValidationRule diffValidationRule = new CliqueDifficultyValidationRule();
     assertThat(diffValidationRule.validate(newBlock, parentHeader, cliqueProtocolContext))
@@ -114,15 +135,14 @@ public class CliqueDifficultyValidationRuleTest {
 
   @Test
   public void isFalseIfInTurnValidatorSuppliesDifficultyOfOne() {
+    final long IN_TURN_BLOCK_NUMBER = validatorList.size();
     final UInt256 REPORTED_DIFFICULTY = UInt256.of(1);
 
-    final BlockHeader parentHeader =
+    blockHeaderBuilder.number(IN_TURN_BLOCK_NUMBER - 1L);
-        TestHelpers.createCliqueSignedBlockHeader(blockHeaderBuilder, otherKeyPair, validatorList);
+    final BlockHeader parentHeader = createCliqueSignedBlock(blockHeaderBuilder);
 
-    blockHeaderBuilder.difficulty(REPORTED_DIFFICULTY);
+    blockHeaderBuilder.number(IN_TURN_BLOCK_NUMBER).difficulty(REPORTED_DIFFICULTY);
-    final BlockHeader newBlock =
+    final BlockHeader newBlock = createCliqueSignedBlock(blockHeaderBuilder);
-        TestHelpers.createCliqueSignedBlockHeader(
-            blockHeaderBuilder, proposerKeyPair, validatorList);
 
     final CliqueDifficultyValidationRule diffValidationRule = new CliqueDifficultyValidationRule();
     assertThat(diffValidationRule.validate(newBlock, parentHeader, cliqueProtocolContext))

consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSignersAtHashTest.java

@@ -37,9 +37,7 @@ import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessRe
 import tech.pegasys.pantheon.util.bytes.BytesValue;
 
 import java.util.List;
-import java.util.NavigableSet;
 import java.util.Optional;
-import java.util.TreeSet;
 
 import org.assertj.core.api.AssertionsForClassTypes;
 import org.bouncycastle.util.encoders.Hex;
@@ -54,7 +52,7 @@ public class CliqueGetSignersAtHashTest {
 
   private CliqueGetSignersAtHash method;
   private BlockHeader blockHeader;
-  private NavigableSet<Address> validators;
+  private List<Address> validators;
   private List<String> validatorsAsStrings;
 
   @Mock private BlockchainQueries blockchainQueries;
@@ -77,11 +75,10 @@ public class CliqueGetSignersAtHashTest {
     blockHeader = blockHeaderTestFixture.extraData(bufferToInject).buildHeader();
 
     validators =
-        new TreeSet<>(
+        asList(
-            asList(
+            fromHexString("0x42eb768f2244c8811c63729a21a3569731535f06"),
-                fromHexString("0x42eb768f2244c8811c63729a21a3569731535f06"),
+            fromHexString("0x7ffc57839b00206d1ad20c69a1981b489f772031"),
-                fromHexString("0x7ffc57839b00206d1ad20c69a1981b489f772031"),
+            fromHexString("0xb279182d99e65703f0076e4812653aab85fca0f0"));
-                fromHexString("0xb279182d99e65703f0076e4812653aab85fca0f0")));
     validatorsAsStrings = validators.stream().map(Object::toString).collect(toList());
   }
 

consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSignersTest.java

@@ -36,9 +36,7 @@ import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessRe
 import tech.pegasys.pantheon.util.bytes.BytesValue;
 
 import java.util.List;
-import java.util.NavigableSet;
 import java.util.Optional;
-import java.util.TreeSet;
 
 import org.bouncycastle.util.encoders.Hex;
 import org.junit.Before;
@@ -51,7 +49,7 @@ import org.mockito.junit.MockitoJUnitRunner;
 public class CliqueGetSignersTest {
   private CliqueGetSigners method;
   private BlockHeader blockHeader;
-  private NavigableSet<Address> validators;
+  private List<Address> validators;
   private List<String> validatorAsStrings;
 
   @Mock private BlockchainQueries blockchainQueries;
@@ -71,11 +69,10 @@ public class CliqueGetSignersTest {
     blockHeader = blockHeaderTestFixture.extraData(bufferToInject).buildHeader();
 
     validators =
-        new TreeSet<>(
+        asList(
-            asList(
+            fromHexString("0x42eb768f2244c8811c63729a21a3569731535f06"),
-                fromHexString("0x42eb768f2244c8811c63729a21a3569731535f06"),
+            fromHexString("0x7ffc57839b00206d1ad20c69a1981b489f772031"),
-                fromHexString("0x7ffc57839b00206d1ad20c69a1981b489f772031"),
+            fromHexString("0xb279182d99e65703f0076e4812653aab85fca0f0"));
-                fromHexString("0xb279182d99e65703f0076e4812653aab85fca0f0")));
     validatorAsStrings = validators.stream().map(Object::toString).collect(toList());
   }
 

consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/ValidatorProvider.java

@@ -14,10 +14,10 @@ package tech.pegasys.pantheon.consensus.common;
 
 import tech.pegasys.pantheon.ethereum.core.Address;
 
-import java.util.NavigableSet;
+import java.util.Collection;
 
 public interface ValidatorProvider {
 
   // Returns the current list of validators
-  NavigableSet<Address> getCurrentValidators();
+  Collection<Address> getCurrentValidators();
 }

consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/VoteTally.java

@@ -20,9 +20,9 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.NavigableSet;
 import java.util.Optional;
 import java.util.Set;
+import java.util.SortedSet;
 import java.util.TreeSet;
 
 import com.google.common.collect.Maps;
@@ -30,7 +30,7 @@ import com.google.common.collect.Maps;
 /** Tracks the current list of validators and votes to add or drop validators. */
 public class VoteTally implements ValidatorProvider {
 
-  private final NavigableSet<Address> currentValidators;
+  private final SortedSet<Address> currentValidators;
 
   private final Map<Address, Set<Address>> addVotesBySubject;
   private final Map<Address, Set<Address>> removeVotesBySubject;
@@ -108,7 +108,7 @@ public class VoteTally implements ValidatorProvider {
   }
 
   @Override
-  public NavigableSet<Address> getCurrentValidators() {
+  public Collection<Address> getCurrentValidators() {
     return currentValidators;
   }
 

consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java

@@ -32,8 +32,6 @@ import tech.pegasys.pantheon.util.bytes.BytesValue;
 
 import java.math.BigInteger;
 import java.util.List;
-import java.util.NavigableSet;
-import java.util.TreeSet;
 
 import com.google.common.collect.Lists;
 import org.junit.Before;
@@ -44,7 +42,7 @@ import org.mockito.junit.MockitoJUnitRunner;
 @RunWith(MockitoJUnitRunner.class)
 public class IbftNetworkPeersTest {
 
-  private final NavigableSet<Address> validators = new TreeSet<>();
+  private final List<Address> validators = Lists.newArrayList();
   private final List<PublicKey> publicKeys = Lists.newArrayList();
 
   private final List<PeerConnection> peerConnections = Lists.newArrayList();