From 64d80a52cb7f9c24b38c8b0dc9d719a7c13cf46c Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Fri, 30 Nov 2018 14:46:04 -0700 Subject: [PATCH] =?UTF-8?q?Revert=20"Repair=20Clique=20Proposer=20Selectio?= =?UTF-8?q?n"=20#339=20-=20Breaks=20G=C3=B6rli=20testnet=20(#343)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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/CliqueBlockHashing.java | 2 +- .../blockcreation/CliqueProposerSelector.java | 29 ++----- .../CliqueDifficultyCalculatorTest.java | 20 ++--- .../CliqueBlockSchedulerTest.java | 28 ++++--- .../CliqueProposerSelectorTest.java | 71 ++++------------- .../CliqueDifficultyValidationRuleTest.java | 78 ++++++++++++------- .../methods/CliqueGetSignersAtHashTest.java | 13 ++-- .../jsonrpc/methods/CliqueGetSignersTest.java | 13 ++-- .../consensus/common/ValidatorProvider.java | 4 +- .../pantheon/consensus/common/VoteTally.java | 6 +- .../ibft/network/IbftNetworkPeersTest.java | 4 +- 11 files changed, 110 insertions(+), 158 deletions(-) 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 a7579cfbbf..de342cfa2f 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 4d91fd14ad..282edc5056 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,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.SortedSet; +import java.util.ArrayList; +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
validatorSet = parentVoteTally.getCurrentValidators(); + final List
validatorSet = new ArrayList<>(parentVoteTally.getCurrentValidators()); - Address prevBlockProposer = validatorSet.first(); - if (parentHeader.getNumber() != BlockHeader.GENESIS_BLOCK_NUMBER) { - prevBlockProposer = blockInterface.getProposerOfBlock(parentHeader); - } + final long nextBlockNumber = parentHeader.getNumber() + 1L; + final int indexIntoValidators = (int) (nextBlockNumber % validatorSet.size()); - 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(); - } + return validatorSet.get(indexIntoValidators); } } 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 79dd394e5b..ad11565b1e 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,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() { - // The proposer created the last block, so the next block must be a difficulty of 1. + public void inTurnValidatorProducesDifficultyOfTwo() { final CliqueDifficultyCalculator calculator = new CliqueDifficultyCalculator(localAddr); - BlockHeader parentHeader = - createCliqueSignedBlockHeader(blockHeaderBuilder, proposerKeyPair, validatorList); + final BlockHeader parentHeader = blockHeaderBuilder.number(1).buildHeader(); assertThat(calculator.nextDifficulty(0, parentHeader, cliqueProtocolContext)) - .isEqualTo(BigInteger.valueOf(1)); + .isEqualTo(BigInteger.valueOf(2)); } @Test - public void inTurnValidatorCreatesDifficultyOfTwo() { - final CliqueDifficultyCalculator calculator = - new CliqueDifficultyCalculator(validatorList.get(1)); // i.e. not the proposer. + public void outTurnValidatorProducesDifficultyOfOne() { + final CliqueDifficultyCalculator calculator = new CliqueDifficultyCalculator(localAddr); - BlockHeader parentHeader = - createCliqueSignedBlockHeader(blockHeaderBuilder, proposerKeyPair, validatorList); + final BlockHeader parentHeader = blockHeaderBuilder.number(2).buildHeader(); assertThat(calculator.nextDifficulty(0, parentHeader, cliqueProtocolContext)) - .isEqualTo(BigInteger.valueOf(2)); + .isEqualTo(BigInteger.valueOf(1)); } } 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 28b66e9f6d..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 @@ -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 otherAddr = AddressHelpers.calculateAddressWithRespectTo(localAddr, 1); + private Address localAddr; private final List
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); 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 7779831bb0..7427881088 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,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
validatorList = - Lists.newArrayList( - AddressHelpers.calculateAddressWithRespectTo(proposerAddress, -1), - proposerAddress, - AddressHelpers.calculateAddressWithRespectTo(proposerAddress, 1), - AddressHelpers.calculateAddressWithRespectTo(proposerAddress, 2)); + Arrays.asList( + AddressHelpers.ofValue(1), + AddressHelpers.ofValue(2), + AddressHelpers.ofValue(3), + 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
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())); + } } } 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 555fb9f746..4bbdd40cd1 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,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
validatorList = Lists.newArrayList(); private ProtocolContext cliqueProtocolContext; private BlockHeaderTestFixture blockHeaderBuilder; @Before public void setup() { - validatorList.add(Util.publicKeyToAddress(proposerKeyPair.getPublicKey())); - validatorList.add(Util.publicKeyToAddress(otherKeyPair.getPublicKey())); + final Address localAddress = Util.publicKeyToAddress(proposerKeyPair.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 = - TestHelpers.createCliqueSignedBlockHeader(blockHeaderBuilder, otherKeyPair, validatorList); + blockHeaderBuilder.number(IN_TURN_BLOCK_NUMBER - 1L); + final BlockHeader parentHeader = createCliqueSignedBlock(blockHeaderBuilder); blockHeaderBuilder.number(IN_TURN_BLOCK_NUMBER).difficulty(REPORTED_DIFFICULTY); - final BlockHeader newBlock = - TestHelpers.createCliqueSignedBlockHeader( - blockHeaderBuilder, proposerKeyPair, validatorList); + final BlockHeader newBlock = createCliqueSignedBlock(blockHeaderBuilder); 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 = - TestHelpers.createCliqueSignedBlockHeader( - blockHeaderBuilder, proposerKeyPair, validatorList); + blockHeaderBuilder.number(OUT_OF_TURN_BLOCK_NUMBER - 1L); + final BlockHeader parentHeader = createCliqueSignedBlock(blockHeaderBuilder); blockHeaderBuilder.number(OUT_OF_TURN_BLOCK_NUMBER).difficulty(REPORTED_DIFFICULTY); - final BlockHeader newBlock = - TestHelpers.createCliqueSignedBlockHeader( - blockHeaderBuilder, proposerKeyPair, validatorList); + final BlockHeader newBlock = createCliqueSignedBlock(blockHeaderBuilder); 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 = - TestHelpers.createCliqueSignedBlockHeader( - blockHeaderBuilder, proposerKeyPair, validatorList); + blockHeaderBuilder.number(OUT_OF_TURN_BLOCK_NUMBER - 1L); + final BlockHeader parentHeader = createCliqueSignedBlock(blockHeaderBuilder); - blockHeaderBuilder.difficulty(REPORTED_DIFFICULTY); - final BlockHeader newBlock = - TestHelpers.createCliqueSignedBlockHeader( - blockHeaderBuilder, proposerKeyPair, validatorList); + blockHeaderBuilder.number(OUT_OF_TURN_BLOCK_NUMBER).difficulty(REPORTED_DIFFICULTY); + final BlockHeader newBlock = createCliqueSignedBlock(blockHeaderBuilder); 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 = - TestHelpers.createCliqueSignedBlockHeader(blockHeaderBuilder, otherKeyPair, validatorList); + blockHeaderBuilder.number(IN_TURN_BLOCK_NUMBER - 1L); + final BlockHeader parentHeader = createCliqueSignedBlock(blockHeaderBuilder); - blockHeaderBuilder.difficulty(REPORTED_DIFFICULTY); - final BlockHeader newBlock = - TestHelpers.createCliqueSignedBlockHeader( - blockHeaderBuilder, proposerKeyPair, validatorList); + blockHeaderBuilder.number(IN_TURN_BLOCK_NUMBER).difficulty(REPORTED_DIFFICULTY); + final BlockHeader newBlock = createCliqueSignedBlock(blockHeaderBuilder); final CliqueDifficultyValidationRule diffValidationRule = new CliqueDifficultyValidationRule(); assertThat(diffValidationRule.validate(newBlock, parentHeader, cliqueProtocolContext)) diff --git a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSignersAtHashTest.java b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSignersAtHashTest.java index 49a90d1535..1a8b1f3b91 100644 --- a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSignersAtHashTest.java +++ b/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
validators; + private List
validators; private List validatorsAsStrings; @Mock private BlockchainQueries blockchainQueries; @@ -77,11 +75,10 @@ public class CliqueGetSignersAtHashTest { blockHeader = blockHeaderTestFixture.extraData(bufferToInject).buildHeader(); validators = - new TreeSet<>( - asList( - fromHexString("0x42eb768f2244c8811c63729a21a3569731535f06"), - fromHexString("0x7ffc57839b00206d1ad20c69a1981b489f772031"), - fromHexString("0xb279182d99e65703f0076e4812653aab85fca0f0"))); + asList( + fromHexString("0x42eb768f2244c8811c63729a21a3569731535f06"), + fromHexString("0x7ffc57839b00206d1ad20c69a1981b489f772031"), + fromHexString("0xb279182d99e65703f0076e4812653aab85fca0f0")); validatorsAsStrings = validators.stream().map(Object::toString).collect(toList()); } diff --git a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSignersTest.java b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSignersTest.java index 417bf10675..5b98d517ef 100644 --- a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSignersTest.java +++ b/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
validators; + private List
validators; private List validatorAsStrings; @Mock private BlockchainQueries blockchainQueries; @@ -71,11 +69,10 @@ public class CliqueGetSignersTest { blockHeader = blockHeaderTestFixture.extraData(bufferToInject).buildHeader(); validators = - new TreeSet<>( - asList( - fromHexString("0x42eb768f2244c8811c63729a21a3569731535f06"), - fromHexString("0x7ffc57839b00206d1ad20c69a1981b489f772031"), - fromHexString("0xb279182d99e65703f0076e4812653aab85fca0f0"))); + asList( + fromHexString("0x42eb768f2244c8811c63729a21a3569731535f06"), + fromHexString("0x7ffc57839b00206d1ad20c69a1981b489f772031"), + fromHexString("0xb279182d99e65703f0076e4812653aab85fca0f0")); validatorAsStrings = validators.stream().map(Object::toString).collect(toList()); } diff --git a/consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/ValidatorProvider.java b/consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/ValidatorProvider.java index 2ce53ea3f2..6f358dd788 100644 --- a/consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/ValidatorProvider.java +++ b/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
getCurrentValidators(); + Collection
getCurrentValidators(); } diff --git a/consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/VoteTally.java b/consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/VoteTally.java index 55b9e7f7be..c274131b77 100644 --- a/consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/VoteTally.java +++ b/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
currentValidators; + private final SortedSet
currentValidators; private final Map> addVotesBySubject; private final Map> removeVotesBySubject; @@ -108,7 +108,7 @@ public class VoteTally implements ValidatorProvider { } @Override - public NavigableSet
getCurrentValidators() { + public Collection
getCurrentValidators() { return currentValidators; } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java index 7a878811e6..5297447e7b 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java +++ b/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
validators = new TreeSet<>(); + private final List
validators = Lists.newArrayList(); private final List publicKeys = Lists.newArrayList(); private final List peerConnections = Lists.newArrayList();