From aac4fd40c56466d4912e5c5cd5577962ded64d44 Mon Sep 17 00:00:00 2001 From: tmohay <37158202+rain-on@users.noreply.github.com> Date: Tue, 4 Dec 2018 18:02:45 +1100 Subject: [PATCH] Repair VoteTallyCache incorrectly applying vote to parent (#360) A problem was identified whereby the vote in a child block was applied to the parent, this was resolved by duplicating the parent VoteTally before modifying it, and injecting to the child block. Some refactoring was also conducted to make VoteTally logic simpler. --- .../consensus/clique/CliqueHelpers.java | 6 +- .../consensus/clique/VoteTallyCache.java | 36 +++++++---- .../blockcreation/CliqueBlockCreator.java | 3 +- .../blockcreation/CliqueBlockScheduler.java | 4 +- .../blockcreation/CliqueMinerExecutor.java | 7 ++- .../blockcreation/CliqueProposerSelector.java | 4 +- .../CliqueExtraDataValidationRule.java | 4 +- .../jsonrpc/methods/CliqueGetSigners.java | 2 +- .../methods/CliqueGetSignersAtHash.java | 2 +- .../CliqueDifficultyCalculatorTest.java | 2 +- .../clique/NodeCanProduceNextBlockTest.java | 14 ++--- .../consensus/clique/VoteTallyCacheTest.java | 54 ++++++++++++++--- .../blockcreation/CliqueBlockCreatorTest.java | 2 +- .../CliqueBlockSchedulerTest.java | 2 +- .../CliqueMinerExecutorTest.java | 2 +- .../CliqueProposerSelectorTest.java | 2 +- .../CliqueDifficultyValidationRuleTest.java | 2 +- .../CliqueExtraDataValidationRuleTest.java | 2 +- .../methods/CliqueGetSignersAtHashTest.java | 4 +- .../jsonrpc/methods/CliqueGetSignersTest.java | 8 +-- .../consensus/common/ValidatorProvider.java | 2 +- .../consensus/common/VoteProposer.java | 2 +- .../pantheon/consensus/common/VoteTally.java | 6 +- .../consensus/common/VoteTallyTest.java | 60 +++++++++---------- .../common/VoteTallyUpdaterTest.java | 6 +- .../IbftBlockCreatorFactory.java | 2 +- .../ibft/blockcreation/ProposerSelector.java | 6 +- .../IbftCoinbaseValidationRule.java | 2 +- .../IbftExtraDataValidationRule.java | 2 +- .../ibft/network/IbftNetworkPeers.java | 2 +- .../ibft/network/IbftNetworkPeersTest.java | 4 +- .../IbftExtraDataCalculator.java | 2 +- .../IbftExtraDataValidationRule.java | 2 +- 33 files changed, 156 insertions(+), 104 deletions(-) diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueHelpers.java b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueHelpers.java index b81900d3b0..a38ed4caad 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueHelpers.java +++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueHelpers.java @@ -38,9 +38,9 @@ public class CliqueHelpers { final ProtocolContext protocolContext, final BlockHeader parent) { final VoteTally validatorProvider = - protocolContext.getConsensusState().getVoteTallyCache().getVoteTallyAtBlock(parent); + protocolContext.getConsensusState().getVoteTallyCache().getVoteTallyAfterBlock(parent); - if (!validatorProvider.getCurrentValidators().contains(candidate)) { + if (!validatorProvider.getValidators().contains(candidate)) { return false; } @@ -72,7 +72,7 @@ public class CliqueHelpers { } private static int minimumBlocksSincePreviousSigning(final ValidatorProvider validatorProvider) { - final int validatorCount = validatorProvider.getCurrentValidators().size(); + final int validatorCount = validatorProvider.getValidators().size(); // The number of contiguous blocks in which a signer may only sign 1 (as taken from clique spec) final int signerLimit = (validatorCount / 2) + 1; return signerLimit - 1; diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/VoteTallyCache.java b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/VoteTallyCache.java index 02ad1012ea..4783e7b350 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/VoteTallyCache.java +++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/VoteTallyCache.java @@ -50,7 +50,18 @@ public class VoteTallyCache { this.epochManager = epochManager; } - public VoteTally getVoteTallyAtBlock(final BlockHeader header) { + /** + * Determines the VoteTally for a given block header, by back-tracing the blockchain to a + * previously cached value or epoch block. Then appyling votes in each intermediate header such + * that representative state can be provided. This function assumes the vote cast in {@code + * header} is applied, thus the voteTally returned contains the group of validators who are + * permitted to partake in the next block's creation. + * + * @param header the header of the block after which the VoteTally is to be returned + * @return The Vote Tally (and therefore validators) following the application of all votes upto + * and including the requested header. + */ + public VoteTally getVoteTallyAfterBlock(final BlockHeader header) { try { return voteTallyCache.get(header.getHash(), () -> populateCacheUptoAndIncluding(header)); } catch (final ExecutionException ex) { @@ -64,7 +75,8 @@ public class VoteTallyCache { VoteTally voteTally = null; while (true) { // Will run into an epoch block (and thus a VoteTally) to break loop. - voteTally = findMostRecentAvailableVoteTally(header, intermediateBlocks); + intermediateBlocks.push(header); + voteTally = getValidatorsAfter(header); if (voteTally != null) { break; } @@ -80,25 +92,23 @@ public class VoteTallyCache { return constructMissingCacheEntries(intermediateBlocks, voteTally); } - private VoteTally findMostRecentAvailableVoteTally( - final BlockHeader header, final Deque intermediateBlockHeaders) { - intermediateBlockHeaders.push(header); - VoteTally voteTally = voteTallyCache.getIfPresent(header.getParentHash()); - if ((voteTally == null) && (epochManager.isEpochBlock(header.getNumber()))) { - final CliqueExtraData extraData = CliqueExtraData.decode(header.getExtraData()); - voteTally = new VoteTally(extraData.getValidators()); + private VoteTally getValidatorsAfter(final BlockHeader header) { + if (epochManager.isEpochBlock(header.getNumber())) { + final CliqueBlockInterface blockInterface = new CliqueBlockInterface(); + return new VoteTally(blockInterface.validatorsInBlock(header)); } - return voteTally; + return voteTallyCache.getIfPresent(header.getParentHash()); } private VoteTally constructMissingCacheEntries( final Deque headers, final VoteTally tally) { + final VoteTally mutableVoteTally = tally.copy(); while (!headers.isEmpty()) { final BlockHeader h = headers.pop(); - voteTallyUpdater.updateForBlock(h, tally); - voteTallyCache.put(h.getHash(), tally.copy()); + voteTallyUpdater.updateForBlock(h, mutableVoteTally); + voteTallyCache.put(h.getHash(), mutableVoteTally.copy()); } - return tally; + return mutableVoteTally; } } diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockCreator.java b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockCreator.java index 861f2f551f..c1903486c3 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockCreator.java +++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockCreator.java @@ -85,7 +85,8 @@ public class CliqueBlockCreator extends AbstractBlockCreator { .blockHashFunction(blockHashFunction); final CliqueContext cliqueContext = protocolContext.getConsensusState(); - final VoteTally voteTally = cliqueContext.getVoteTallyCache().getVoteTallyAtBlock(parentHeader); + final VoteTally voteTally = + cliqueContext.getVoteTallyCache().getVoteTallyAfterBlock(parentHeader); final Optional vote = cliqueContext 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 9def5ea0c0..9e63361f5f 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 @@ -60,11 +60,11 @@ public class CliqueBlockScheduler extends DefaultBlockScheduler { if (nextProposer.equals(localNodeAddress)) { return 0; } - return calculatorOutOfTurnDelay(voteTallyCache.getVoteTallyAtBlock(parentHeader)); + return calculatorOutOfTurnDelay(voteTallyCache.getVoteTallyAfterBlock(parentHeader)); } private int calculatorOutOfTurnDelay(final ValidatorProvider validators) { - final int countSigners = validators.getCurrentValidators().size(); + final int countSigners = validators.getValidators().size(); final double multiplier = (countSigners / 2d) + 1; final int maxDelay = (int) (multiplier * OUT_OF_TURN_DELAY_MULTIPLIER_MILLIS); return r.nextInt(maxDelay) + 1; diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMinerExecutor.java b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMinerExecutor.java index 6d0e683143..7de025bdcb 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMinerExecutor.java +++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMinerExecutor.java @@ -101,8 +101,11 @@ public class CliqueMinerExecutor extends AbstractMinerExecutor validatorSet = new ArrayList<>(parentVoteTally.getCurrentValidators()); + final VoteTally parentVoteTally = voteTallyCache.getVoteTallyAfterBlock(parentHeader); + final List
validatorSet = new ArrayList<>(parentVoteTally.getValidators()); final long nextBlockNumber = parentHeader.getNumber() + 1L; final int indexIntoValidators = (int) (nextBlockNumber % validatorSet.size()); diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/headervalidationrules/CliqueExtraDataValidationRule.java b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/headervalidationrules/CliqueExtraDataValidationRule.java index 054a3d275a..f102f83575 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/headervalidationrules/CliqueExtraDataValidationRule.java +++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/headervalidationrules/CliqueExtraDataValidationRule.java @@ -59,9 +59,9 @@ public class CliqueExtraDataValidationRule final ProtocolContext protocolContext) { try { final VoteTally validatorProvider = - protocolContext.getConsensusState().getVoteTallyCache().getVoteTallyAtBlock(parent); + protocolContext.getConsensusState().getVoteTallyCache().getVoteTallyAfterBlock(parent); - final Collection
storedValidators = validatorProvider.getCurrentValidators(); + final Collection
storedValidators = validatorProvider.getValidators(); return extraDataIsValid(storedValidators, header); } catch (final RLPException ex) { diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSigners.java b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSigners.java index 367e977cfc..260d54d73c 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSigners.java +++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSigners.java @@ -53,7 +53,7 @@ public class CliqueGetSigners implements JsonRpcMethod { public JsonRpcResponse response(final JsonRpcRequest request) { final Optional blockHeader = determineBlockHeader(request); return blockHeader - .map(bh -> voteTallyCache.getVoteTallyAtBlock(bh).getCurrentValidators()) + .map(bh -> voteTallyCache.getVoteTallyAfterBlock(bh).getValidators()) .map(addresses -> addresses.stream().map(Objects::toString).collect(Collectors.toList())) .map(addresses -> new JsonRpcSuccessResponse(request.getId(), addresses)) .orElse(new JsonRpcErrorResponse(request.getId(), JsonRpcError.INTERNAL_ERROR)); diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSignersAtHash.java b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSignersAtHash.java index 7fa6c46079..c3ea1d2dda 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSignersAtHash.java +++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/methods/CliqueGetSignersAtHash.java @@ -53,7 +53,7 @@ public class CliqueGetSignersAtHash implements JsonRpcMethod { public JsonRpcResponse response(final JsonRpcRequest request) { final Optional blockHeader = determineBlockHeader(request); return blockHeader - .map(bh -> voteTallyCache.getVoteTallyAtBlock(bh).getCurrentValidators()) + .map(bh -> voteTallyCache.getVoteTallyAfterBlock(bh).getValidators()) .map(addresses -> addresses.stream().map(Objects::toString).collect(Collectors.toList())) .map(addresses -> new JsonRpcSuccessResponse(request.getId(), addresses)) .orElse(new JsonRpcErrorResponse(request.getId(), JsonRpcError.INTERNAL_ERROR)); 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..01744fc1cb 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 @@ -51,7 +51,7 @@ public class CliqueDifficultyCalculatorTest { validatorList.add(AddressHelpers.calculateAddressWithRespectTo(localAddr, 1)); final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); - when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList)); + when(voteTallyCache.getVoteTallyAfterBlock(any())).thenReturn(new VoteTally(validatorList)); final VoteProposer voteProposer = new VoteProposer(); final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, voteProposer, null); diff --git a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/NodeCanProduceNextBlockTest.java b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/NodeCanProduceNextBlockTest.java index c788f26015..a057a34b58 100644 --- a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/NodeCanProduceNextBlockTest.java +++ b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/NodeCanProduceNextBlockTest.java @@ -73,7 +73,7 @@ public class NodeCanProduceNextBlockTest { blockChain = createInMemoryBlockchain(genesisBlock); final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); - when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList)); + when(voteTallyCache.getVoteTallyAfterBlock(any())).thenReturn(new VoteTally(validatorList)); final VoteProposer voteProposer = new VoteProposer(); final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, voteProposer, null); cliqueProtocolContext = new ProtocolContext<>(blockChain, null, cliqueContext); @@ -98,7 +98,7 @@ public class NodeCanProduceNextBlockTest { blockChain = createInMemoryBlockchain(genesisBlock); final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); - when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList)); + when(voteTallyCache.getVoteTallyAfterBlock(any())).thenReturn(new VoteTally(validatorList)); final VoteProposer voteProposer = new VoteProposer(); final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, voteProposer, null); cliqueProtocolContext = new ProtocolContext<>(blockChain, null, cliqueContext); @@ -132,7 +132,7 @@ public class NodeCanProduceNextBlockTest { blockChain = createInMemoryBlockchain(genesisBlock); final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); - when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList)); + when(voteTallyCache.getVoteTallyAfterBlock(any())).thenReturn(new VoteTally(validatorList)); final VoteProposer voteProposer = new VoteProposer(); final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, voteProposer, null); cliqueProtocolContext = new ProtocolContext<>(blockChain, null, cliqueContext); @@ -162,7 +162,7 @@ public class NodeCanProduceNextBlockTest { blockChain = createInMemoryBlockchain(genesisBlock); final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); - when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList)); + when(voteTallyCache.getVoteTallyAfterBlock(any())).thenReturn(new VoteTally(validatorList)); final VoteProposer voteProposer = new VoteProposer(); final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, voteProposer, null); cliqueProtocolContext = new ProtocolContext<>(blockChain, null, cliqueContext); @@ -207,7 +207,7 @@ public class NodeCanProduceNextBlockTest { blockChain = createInMemoryBlockchain(genesisBlock); final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); - when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList)); + when(voteTallyCache.getVoteTallyAfterBlock(any())).thenReturn(new VoteTally(validatorList)); final VoteProposer voteProposer = new VoteProposer(); final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, voteProposer, null); cliqueProtocolContext = new ProtocolContext<>(blockChain, null, cliqueContext); @@ -236,7 +236,7 @@ public class NodeCanProduceNextBlockTest { blockChain = createInMemoryBlockchain(genesisBlock); final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); - when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList)); + when(voteTallyCache.getVoteTallyAfterBlock(any())).thenReturn(new VoteTally(validatorList)); final VoteProposer voteProposer = new VoteProposer(); final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, voteProposer, null); cliqueProtocolContext = new ProtocolContext<>(blockChain, null, cliqueContext); @@ -260,7 +260,7 @@ public class NodeCanProduceNextBlockTest { blockChain = createInMemoryBlockchain(genesisBlock); final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); - when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList)); + when(voteTallyCache.getVoteTallyAfterBlock(any())).thenReturn(new VoteTally(validatorList)); final VoteProposer voteProposer = new VoteProposer(); final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, voteProposer, null); cliqueProtocolContext = new ProtocolContext<>(blockChain, null, cliqueContext); diff --git a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/VoteTallyCacheTest.java b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/VoteTallyCacheTest.java index b85e99685e..954a07548d 100644 --- a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/VoteTallyCacheTest.java +++ b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/VoteTallyCacheTest.java @@ -20,12 +20,17 @@ import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; +import static tech.pegasys.pantheon.consensus.common.VoteType.DROP; import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.createInMemoryBlockchain; import tech.pegasys.pantheon.consensus.common.EpochManager; +import tech.pegasys.pantheon.consensus.common.ValidatorVote; +import tech.pegasys.pantheon.consensus.common.VoteTally; import tech.pegasys.pantheon.consensus.common.VoteTallyUpdater; import tech.pegasys.pantheon.crypto.SECP256K1.Signature; import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain; +import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.AddressHelpers; import tech.pegasys.pantheon.ethereum.core.Block; import tech.pegasys.pantheon.ethereum.core.BlockBody; @@ -36,6 +41,8 @@ import tech.pegasys.pantheon.util.bytes.BytesValue; import java.math.BigInteger; import java.util.Arrays; +import java.util.List; +import java.util.Optional; import com.google.common.util.concurrent.UncheckedExecutionException; import org.assertj.core.util.Lists; @@ -58,13 +65,18 @@ public class VoteTallyCacheTest { private Block block_1; private Block block_2; + private final List
validators = Lists.newArrayList(); + @Before public void constructThreeBlockChain() { + for (int i = 0; i < 3; i++) { + validators.add(AddressHelpers.ofValue(i)); + } headerBuilder.extraData( new CliqueExtraData( BytesValue.wrap(new byte[32]), Signature.create(BigInteger.TEN, BigInteger.TEN, (byte) 1), - Lists.emptyList()) + validators) .encode()); genesisBlock = createEmptyBlock(0, Hash.ZERO); @@ -87,7 +99,7 @@ public class VoteTallyCacheTest { // The votetallyUpdater should be invoked for the requested block, and all parents including // the epoch (genesis) block. final ArgumentCaptor varArgs = ArgumentCaptor.forClass(BlockHeader.class); - cache.getVoteTallyAtBlock(block_2.getHeader()); + cache.getVoteTallyAfterBlock(block_2.getHeader()); verify(tallyUpdater, times(3)).updateForBlock(varArgs.capture(), any()); assertThat(varArgs.getAllValues()) .isEqualTo( @@ -97,10 +109,10 @@ public class VoteTallyCacheTest { // Requesting the vote tally to the parent block should not invoke the voteTallyUpdater as the // vote tally was cached from previous operation. - cache.getVoteTallyAtBlock(block_1.getHeader()); + cache.getVoteTallyAfterBlock(block_1.getHeader()); verifyZeroInteractions(tallyUpdater); - cache.getVoteTallyAtBlock(block_2.getHeader()); + cache.getVoteTallyAfterBlock(block_2.getHeader()); verifyZeroInteractions(tallyUpdater); } @@ -113,7 +125,7 @@ public class VoteTallyCacheTest { final Block orphanBlock = createEmptyBlock(4, Hash.ZERO); assertThatExceptionOfType(UncheckedExecutionException.class) - .isThrownBy(() -> cache.getVoteTallyAtBlock(orphanBlock.getHeader())) + .isThrownBy(() -> cache.getVoteTallyAfterBlock(orphanBlock.getHeader())) .withMessageContaining( "Supplied block was on a orphaned chain, unable to generate " + "VoteTally."); } @@ -125,14 +137,14 @@ public class VoteTallyCacheTest { new VoteTallyCache(blockChain, tallyUpdater, new EpochManager(30_000)); // Load the Cache up to block_2 - cache.getVoteTallyAtBlock(block_2.getHeader()); + cache.getVoteTallyAfterBlock(block_2.getHeader()); reset(tallyUpdater); // Append new blocks to the chain, and ensure the walkback only goes as far as block_2. final Block block_3 = createEmptyBlock(4, block_2.getHeader().getHash()); // Load the Cache up to block_2 - cache.getVoteTallyAtBlock(block_3.getHeader()); + cache.getVoteTallyAfterBlock(block_3.getHeader()); // The votetallyUpdater should be invoked for the requested block, and all parents including // the epoch (genesis) block. @@ -140,4 +152,32 @@ public class VoteTallyCacheTest { verify(tallyUpdater, times(1)).updateForBlock(varArgs.capture(), any()); assertThat(varArgs.getAllValues()).isEqualTo(Arrays.asList(block_3.getHeader())); } + + // A bug was identified in VoteTallyCache whereby a vote cast in the next block *could* be applied + // to the parent block (depending on cache creation ordering). This test ensure the problem is + // resolved. + @Test + public void integrationTestingVotesBeingApplied() { + final EpochManager epochManager = new EpochManager(30_000); + final CliqueBlockInterface blockInterface = mock(CliqueBlockInterface.class); + final VoteTallyUpdater tallyUpdater = new VoteTallyUpdater(epochManager, blockInterface); + + when(blockInterface.extractVoteFromHeader(block_1.getHeader())) + .thenReturn(Optional.of(new ValidatorVote(DROP, validators.get(0), validators.get(2)))); + + when(blockInterface.extractVoteFromHeader(block_2.getHeader())) + .thenReturn(Optional.of(new ValidatorVote(DROP, validators.get(1), validators.get(2)))); + + final VoteTallyCache cache = new VoteTallyCache(blockChain, tallyUpdater, epochManager); + + VoteTally voteTally = cache.getVoteTallyAfterBlock(block_1.getHeader()); + assertThat(voteTally.getValidators()).containsAll(validators); + + voteTally = cache.getVoteTallyAfterBlock(block_2.getHeader()); + + assertThat(voteTally.getValidators()).containsExactly(validators.get(0), validators.get(1)); + + voteTally = cache.getVoteTallyAfterBlock(block_1.getHeader()); + assertThat(voteTally.getValidators()).containsAll(validators); + } } diff --git a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockCreatorTest.java b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockCreatorTest.java index 0ed435cd3d..d372c19cb8 100644 --- a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockCreatorTest.java +++ b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockCreatorTest.java @@ -75,7 +75,7 @@ public class CliqueBlockCreatorTest { validatorList.add(otherAddress); final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); - when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList)); + when(voteTallyCache.getVoteTallyAfterBlock(any())).thenReturn(new VoteTally(validatorList)); voteProposer = new VoteProposer(); final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, voteProposer, null); 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..2f7c593be7 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 @@ -51,7 +51,7 @@ public class CliqueBlockSchedulerTest { validatorList.add(AddressHelpers.calculateAddressWithRespectTo(localAddr, 1)); voteTallyCache = mock(VoteTallyCache.class); - when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList)); + when(voteTallyCache.getVoteTallyAfterBlock(any())).thenReturn(new VoteTally(validatorList)); blockHeaderBuilder = new BlockHeaderTestFixture(); } diff --git a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMinerExecutorTest.java b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMinerExecutorTest.java index aa2b0c6772..fd767332ec 100644 --- a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMinerExecutorTest.java +++ b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMinerExecutorTest.java @@ -66,7 +66,7 @@ public class CliqueMinerExecutorTest { validatorList.add(AddressHelpers.calculateAddressWithRespectTo(localAddress, 3)); final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); - when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList)); + when(voteTallyCache.getVoteTallyAfterBlock(any())).thenReturn(new VoteTally(validatorList)); final VoteProposer voteProposer = new VoteProposer(); final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, voteProposer, null); 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..cfc651b3d8 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 @@ -43,7 +43,7 @@ public class CliqueProposerSelectorTest { @Before public void setup() { voteTallyCache = mock(VoteTallyCache.class); - when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(voteTally); + when(voteTallyCache.getVoteTallyAfterBlock(any())).thenReturn(voteTally); } @Test 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..35ed87d45a 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 @@ -56,7 +56,7 @@ public class CliqueDifficultyValidationRuleTest { validatorList.add(AddressHelpers.calculateAddressWithRespectTo(localAddress, 1)); final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); - when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList)); + when(voteTallyCache.getVoteTallyAfterBlock(any())).thenReturn(new VoteTally(validatorList)); final VoteProposer voteProposer = new VoteProposer(); final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, voteProposer, null); diff --git a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/headervalidationrules/CliqueExtraDataValidationRuleTest.java b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/headervalidationrules/CliqueExtraDataValidationRuleTest.java index e0179028e0..cb323495a0 100644 --- a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/headervalidationrules/CliqueExtraDataValidationRuleTest.java +++ b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/headervalidationrules/CliqueExtraDataValidationRuleTest.java @@ -54,7 +54,7 @@ public class CliqueExtraDataValidationRuleTest { validatorList.add(AddressHelpers.calculateAddressWithRespectTo(localAddr, 1)); final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); - when(voteTallyCache.getVoteTallyAtBlock(any())).thenReturn(new VoteTally(validatorList)); + when(voteTallyCache.getVoteTallyAfterBlock(any())).thenReturn(new VoteTally(validatorList)); final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, null, null); cliqueProtocolContext = new ProtocolContext<>(null, null, cliqueContext); 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 1a8b1f3b91..dd173a5334 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 @@ -109,8 +109,8 @@ public class CliqueGetSignersAtHashTest { when(blockchainQueries.blockByHash(Hash.fromHexString(BLOCK_HASH))) .thenReturn(Optional.of(blockWithMetadata)); when(blockWithMetadata.getHeader()).thenReturn(blockHeader); - when(voteTallyCache.getVoteTallyAtBlock(blockHeader)).thenReturn(voteTally); - when(voteTally.getCurrentValidators()).thenReturn(validators); + when(voteTallyCache.getVoteTallyAfterBlock(blockHeader)).thenReturn(voteTally); + when(voteTally.getValidators()).thenReturn(validators); final JsonRpcSuccessResponse response = (JsonRpcSuccessResponse) method.response(request); assertEquals(validatorsAsStrings, response.getResult()); 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 5b98d517ef..0e97e6603c 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 @@ -89,8 +89,8 @@ public class CliqueGetSignersTest { when(blockchainQueries.headBlockNumber()).thenReturn(3065995L); when(blockchainQueries.blockByNumber(3065995L)).thenReturn(Optional.of(blockWithMetadata)); when(blockWithMetadata.getHeader()).thenReturn(blockHeader); - when(voteTallyCache.getVoteTallyAtBlock(blockHeader)).thenReturn(voteTally); - when(voteTally.getCurrentValidators()).thenReturn(validators); + when(voteTallyCache.getVoteTallyAfterBlock(blockHeader)).thenReturn(voteTally); + when(voteTally.getValidators()).thenReturn(validators); final JsonRpcSuccessResponse response = (JsonRpcSuccessResponse) method.response(request); assertEquals(validatorAsStrings, response.getResult()); @@ -104,8 +104,8 @@ public class CliqueGetSignersTest { when(blockchainQueries.blockByNumber(3065995L)).thenReturn(Optional.of(blockWithMetadata)); when(blockWithMetadata.getHeader()).thenReturn(blockHeader); - when(voteTallyCache.getVoteTallyAtBlock(blockHeader)).thenReturn(voteTally); - when(voteTally.getCurrentValidators()).thenReturn(validators); + when(voteTallyCache.getVoteTallyAfterBlock(blockHeader)).thenReturn(voteTally); + when(voteTally.getValidators()).thenReturn(validators); final JsonRpcSuccessResponse response = (JsonRpcSuccessResponse) method.response(request); assertEquals(validatorAsStrings, response.getResult()); 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 6f358dd788..c9da33aa4a 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 @@ -19,5 +19,5 @@ import java.util.Collection; public interface ValidatorProvider { // Returns the current list of validators - Collection
getCurrentValidators(); + Collection
getValidators(); } diff --git a/consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/VoteProposer.java b/consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/VoteProposer.java index f80a9e996a..b91963db95 100644 --- a/consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/VoteProposer.java +++ b/consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/VoteProposer.java @@ -107,7 +107,7 @@ public class VoteProposer { * votes */ public Optional getVote(final Address localAddress, final VoteTally tally) { - final Collection
validators = tally.getCurrentValidators(); + final Collection
validators = tally.getValidators(); final List> validVotes = new ArrayList<>(); proposals 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 c274131b77..e250fc8a78 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 @@ -107,8 +107,12 @@ public class VoteTally implements ValidatorProvider { addVotesBySubject.clear(); } + /** + * @return The collection of validators after the voting at the most recent block has been + * finalised. + */ @Override - public Collection
getCurrentValidators() { + public Collection
getValidators() { return currentValidators; } diff --git a/consensus/common/src/test/java/tech/pegasys/pantheon/consensus/common/VoteTallyTest.java b/consensus/common/src/test/java/tech/pegasys/pantheon/consensus/common/VoteTallyTest.java index 8a6248bd09..8595ab59da 100644 --- a/consensus/common/src/test/java/tech/pegasys/pantheon/consensus/common/VoteTallyTest.java +++ b/consensus/common/src/test/java/tech/pegasys/pantheon/consensus/common/VoteTallyTest.java @@ -41,7 +41,7 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(ADD, validator1, validator5)); voteTally.addVote(new ValidatorVote(ADD, validator2, validator5)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4); } @@ -52,7 +52,7 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(ADD, validator2, validator5)); voteTally.addVote(new ValidatorVote(ADD, validator3, validator5)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4, validator5); } @@ -64,7 +64,7 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(ADD, validator2, validator4)); voteTally.addVote(new ValidatorVote(ADD, validator3, validator4)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4, validator5); } @@ -75,7 +75,7 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(ADD, validator2, validator5)); voteTally.addVote(new ValidatorVote(ADD, validator2, validator5)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4); } @@ -87,7 +87,7 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(ADD, validator2, validator5)); voteTally.addVote(new ValidatorVote(ADD, validator3, validator5)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4); } @@ -99,7 +99,7 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(ADD, validator3, validator5)); voteTally.addVote(new ValidatorVote(DROP, validator1, validator5)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4, validator5); } @@ -111,20 +111,20 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(ADD, validator2, validator5)); voteTally.addVote(new ValidatorVote(ADD, validator3, validator5)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4, validator5); // Then vote it back out voteTally.addVote(new ValidatorVote(DROP, validator2, validator5)); voteTally.addVote(new ValidatorVote(DROP, validator3, validator5)); voteTally.addVote(new ValidatorVote(DROP, validator4, validator5)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4); // And then start voting to add it back in, but validator1's vote should have been discarded voteTally.addVote(new ValidatorVote(ADD, validator2, validator5)); voteTally.addVote(new ValidatorVote(ADD, validator3, validator5)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4); } @@ -133,7 +133,7 @@ public class VoteTallyTest { final VoteTally voteTally = new VoteTally(singletonList(validator1)); voteTally.addVote(new ValidatorVote(ADD, validator1, validator2)); - assertThat(voteTally.getCurrentValidators()).containsExactly(validator1, validator2); + assertThat(voteTally.getValidators()).containsExactly(validator1, validator2); } @Test @@ -141,11 +141,10 @@ public class VoteTallyTest { final VoteTally voteTally = new VoteTally(asList(validator1, validator2)); voteTally.addVote(new ValidatorVote(ADD, validator1, validator3)); - assertThat(voteTally.getCurrentValidators()).containsExactly(validator1, validator2); + assertThat(voteTally.getValidators()).containsExactly(validator1, validator2); voteTally.addVote(new ValidatorVote(ADD, validator2, validator3)); - assertThat(voteTally.getCurrentValidators()) - .containsExactly(validator1, validator2, validator3); + assertThat(voteTally.getValidators()).containsExactly(validator1, validator2, validator3); } @Test @@ -156,7 +155,7 @@ public class VoteTallyTest { voteTally.discardOutstandingVotes(); voteTally.addVote(new ValidatorVote(ADD, validator3, validator5)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4); } @@ -166,7 +165,7 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(DROP, validator1, validator4)); voteTally.addVote(new ValidatorVote(DROP, validator2, validator4)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4); } @@ -177,8 +176,7 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(DROP, validator2, validator4)); voteTally.addVote(new ValidatorVote(DROP, validator3, validator4)); - assertThat(voteTally.getCurrentValidators()) - .containsExactly(validator1, validator2, validator3); + assertThat(voteTally.getValidators()).containsExactly(validator1, validator2, validator3); } @Test @@ -188,8 +186,7 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(DROP, validator2, validator3)); voteTally.addVote(new ValidatorVote(DROP, validator4, validator3)); - assertThat(voteTally.getCurrentValidators()) - .containsExactly(validator1, validator2, validator4); + assertThat(voteTally.getValidators()).containsExactly(validator1, validator2, validator4); } @Test @@ -199,7 +196,7 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(DROP, validator2, validator4)); voteTally.addVote(new ValidatorVote(DROP, validator2, validator4)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4); } @@ -211,7 +208,7 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(DROP, validator2, validator4)); voteTally.addVote(new ValidatorVote(DROP, validator3, validator4)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4); } @@ -223,8 +220,7 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(DROP, validator3, validator4)); voteTally.addVote(new ValidatorVote(ADD, validator1, validator4)); - assertThat(voteTally.getCurrentValidators()) - .containsExactly(validator1, validator2, validator3); + assertThat(voteTally.getValidators()).containsExactly(validator1, validator2, validator3); } @Test @@ -236,16 +232,14 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(DROP, validator1, validator4)); voteTally.addVote(new ValidatorVote(DROP, validator2, validator4)); voteTally.addVote(new ValidatorVote(DROP, validator3, validator4)); - assertThat(voteTally.getCurrentValidators()) - .containsExactly(validator1, validator2, validator3); + assertThat(voteTally.getValidators()).containsExactly(validator1, validator2, validator3); // Now adding only requires 2 votes (>50% of the 3 remaining validators) // but validator4's vote no longer counts voteTally.addVote(new ValidatorVote(ADD, validator1, validator5)); voteTally.addVote(new ValidatorVote(DROP, validator1, validator3)); - assertThat(voteTally.getCurrentValidators()) - .containsExactly(validator1, validator2, validator3); + assertThat(voteTally.getValidators()).containsExactly(validator1, validator2, validator3); } @Test @@ -257,20 +251,20 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(DROP, validator2, validator5)); voteTally.addVote(new ValidatorVote(DROP, validator3, validator5)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4); // Then vote it back in voteTally.addVote(new ValidatorVote(ADD, validator2, validator5)); voteTally.addVote(new ValidatorVote(ADD, validator3, validator5)); voteTally.addVote(new ValidatorVote(ADD, validator4, validator5)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4, validator5); // And then start voting to drop it again, but validator1's vote should have been discarded voteTally.addVote(new ValidatorVote(DROP, validator2, validator5)); voteTally.addVote(new ValidatorVote(DROP, validator3, validator5)); - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4, validator5); } @@ -284,21 +278,21 @@ public class VoteTallyTest { voteTally.addVote(new ValidatorVote(DROP, validator2, validator1)); // Neither vote has enough votes to complete. - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4); voteTally.addVote(new ValidatorVote(ADD, validator3, validator5)); voteTally.addVote(new ValidatorVote(DROP, validator3, validator1)); // Validator 5 now has 3 votes and is added - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator1, validator2, validator3, validator4, validator5); voteTally.addVote(new ValidatorVote(ADD, validator4, validator5)); voteTally.addVote(new ValidatorVote(DROP, validator4, validator1)); // Validator 1 now gets dropped. - assertThat(voteTally.getCurrentValidators()) + assertThat(voteTally.getValidators()) .containsExactly(validator2, validator3, validator4, validator5); } diff --git a/consensus/common/src/test/java/tech/pegasys/pantheon/consensus/common/VoteTallyUpdaterTest.java b/consensus/common/src/test/java/tech/pegasys/pantheon/consensus/common/VoteTallyUpdaterTest.java index 2c384c61bf..63d3fe3100 100644 --- a/consensus/common/src/test/java/tech/pegasys/pantheon/consensus/common/VoteTallyUpdaterTest.java +++ b/consensus/common/src/test/java/tech/pegasys/pantheon/consensus/common/VoteTallyUpdaterTest.java @@ -102,7 +102,7 @@ public class VoteTallyUpdaterTest { when(blockchain.getBlockHeader(EPOCH_LENGTH)).thenReturn(Optional.of(header)); final VoteTally voteTally = updater.buildVoteTallyFromBlockchain(blockchain); - assertThat(voteTally.getCurrentValidators()).containsExactly(subject, validator1); + assertThat(voteTally.getValidators()).containsExactly(subject, validator1); } @Test @@ -115,7 +115,7 @@ public class VoteTallyUpdaterTest { when(blockchain.getBlockHeader(EPOCH_LENGTH)).thenReturn(Optional.of(header)); final VoteTally voteTally = updater.buildVoteTallyFromBlockchain(blockchain); - assertThat(voteTally.getCurrentValidators()).containsExactly(subject, validator1); + assertThat(voteTally.getValidators()).containsExactly(subject, validator1); } @Test @@ -136,6 +136,6 @@ public class VoteTallyUpdaterTest { when(blockchain.getBlockHeader(EPOCH_LENGTH + 1)).thenReturn(Optional.of(voteBlockHeader)); final VoteTally voteTally = updater.buildVoteTallyFromBlockchain(blockchain); - assertThat(voteTally.getCurrentValidators()).containsExactly(subject, validator1); + assertThat(voteTally.getValidators()).containsExactly(subject, validator1); } } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorFactory.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorFactory.java index 60a3d34130..1ccb453790 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorFactory.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorFactory.java @@ -89,7 +89,7 @@ public class IbftBlockCreatorFactory { final Optional proposal = protocolContext.getConsensusState().getVoteProposer().getVote(localAddress, voteTally); - final List
validators = new ArrayList<>(voteTally.getCurrentValidators()); + final List
validators = new ArrayList<>(voteTally.getValidators()); final IbftExtraData extraData = new IbftExtraData( diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/ProposerSelector.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/ProposerSelector.java index 86490f85d2..43426ccb74 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/ProposerSelector.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/ProposerSelector.java @@ -82,7 +82,7 @@ public class ProposerSelector { final long prevBlockNumber = roundIdentifier.getSequenceNumber() - 1; final Address prevBlockProposer = getProposerOfBlock(prevBlockNumber); - if (!validators.getCurrentValidators().contains(prevBlockProposer)) { + if (!validators.getValidators().contains(prevBlockProposer)) { return handleMissingProposer(prevBlockProposer, roundIdentifier); } else { return handleWithExistingProposer(prevBlockProposer, roundIdentifier); @@ -97,7 +97,7 @@ public class ProposerSelector { */ private Address handleMissingProposer( final Address prevBlockProposer, final ConsensusRoundIdentifier roundIdentifier) { - final NavigableSet
validatorSet = new TreeSet<>(validators.getCurrentValidators()); + final NavigableSet
validatorSet = new TreeSet<>(validators.getValidators()); final SortedSet
latterValidators = validatorSet.tailSet(prevBlockProposer, false); final Address nextProposer; if (latterValidators.isEmpty()) { @@ -138,7 +138,7 @@ public class ProposerSelector { */ private Address calculateRoundSpecificValidator( final Address baseProposer, final int indexOffset) { - final List
currentValidatorList = new ArrayList<>(validators.getCurrentValidators()); + final List
currentValidatorList = new ArrayList<>(validators.getValidators()); final int prevProposerIndex = currentValidatorList.indexOf(baseProposer); final int roundValidatorIndex = (prevProposerIndex + indexOffset) % currentValidatorList.size(); return currentValidatorList.get(roundValidatorIndex); diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCoinbaseValidationRule.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCoinbaseValidationRule.java index f2ac4d5f3b..9dd8642355 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCoinbaseValidationRule.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCoinbaseValidationRule.java @@ -41,7 +41,7 @@ public class IbftCoinbaseValidationRule implements AttachedBlockHeaderValidation final ValidatorProvider validatorProvider = context.getConsensusState().getVoteTally(); Address proposer = header.getCoinbase(); - final Collection
storedValidators = validatorProvider.getCurrentValidators(); + final Collection
storedValidators = validatorProvider.getValidators(); if (!storedValidators.contains(proposer)) { LOGGER.trace("Block proposer is not a member of the validators."); diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftExtraDataValidationRule.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftExtraDataValidationRule.java index a8ee59317a..75dd02e41e 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftExtraDataValidationRule.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftExtraDataValidationRule.java @@ -73,7 +73,7 @@ public class IbftExtraDataValidationRule implements AttachedBlockHeaderValidatio final ValidatorProvider validatorProvider = context.getConsensusState().getVoteTally(); final IbftExtraData ibftExtraData = IbftExtraData.decode(header.getExtraData()); - final Collection
storedValidators = validatorProvider.getCurrentValidators(); + final Collection
storedValidators = validatorProvider.getValidators(); if (validateCommitSeals) { final List
committers = diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java index c3c064d598..fa5c15f6e2 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java @@ -53,7 +53,7 @@ public class IbftNetworkPeers { } public void multicastToValidators(final MessageData message) { - final Collection
validators = validatorProvider.getCurrentValidators(); + final Collection
validators = validatorProvider.getValidators(); sendMessageToSpecificAddresses(validators, message); } 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 5297447e7b..3609bdc829 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 @@ -67,7 +67,7 @@ public class IbftNetworkPeersTest { // Only add the first Peer's address to the validators. validators.add(Util.publicKeyToAddress(publicKeys.get(0))); final ValidatorProvider validatorProvider = mock(ValidatorProvider.class); - when(validatorProvider.getCurrentValidators()).thenReturn(validators); + when(validatorProvider.getValidators()).thenReturn(validators); final IbftNetworkPeers peers = new IbftNetworkPeers(validatorProvider); for (final PeerConnection peer : peerConnections) { @@ -88,7 +88,7 @@ public class IbftNetworkPeersTest { validators.add(Util.publicKeyToAddress(publicKeys.get(0))); final ValidatorProvider validatorProvider = mock(ValidatorProvider.class); - when(validatorProvider.getCurrentValidators()).thenReturn(validators); + when(validatorProvider.getValidators()).thenReturn(validators); final IbftNetworkPeers peers = new IbftNetworkPeers(validatorProvider); diff --git a/consensus/ibftlegacy/src/main/java/tech/pegasys/pantheon/consensus/ibftlegacy/blockcreation/IbftExtraDataCalculator.java b/consensus/ibftlegacy/src/main/java/tech/pegasys/pantheon/consensus/ibftlegacy/blockcreation/IbftExtraDataCalculator.java index b94cf5cab1..030d4e3c9d 100644 --- a/consensus/ibftlegacy/src/main/java/tech/pegasys/pantheon/consensus/ibftlegacy/blockcreation/IbftExtraDataCalculator.java +++ b/consensus/ibftlegacy/src/main/java/tech/pegasys/pantheon/consensus/ibftlegacy/blockcreation/IbftExtraDataCalculator.java @@ -36,7 +36,7 @@ public class IbftExtraDataCalculator implements ExtraDataCalculator { vanityData, Lists.newArrayList(), null, - Lists.newArrayList(validatorProvider.getCurrentValidators())); + Lists.newArrayList(validatorProvider.getValidators())); return baseExtraData.encode(); } } diff --git a/consensus/ibftlegacy/src/main/java/tech/pegasys/pantheon/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRule.java b/consensus/ibftlegacy/src/main/java/tech/pegasys/pantheon/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRule.java index a756c9bae7..b1aaf78e62 100644 --- a/consensus/ibftlegacy/src/main/java/tech/pegasys/pantheon/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRule.java +++ b/consensus/ibftlegacy/src/main/java/tech/pegasys/pantheon/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRule.java @@ -74,7 +74,7 @@ public class IbftExtraDataValidationRule implements AttachedBlockHeaderValidatio final Address proposer = IbftBlockHashing.recoverProposerAddress(header, ibftExtraData); - final Collection
storedValidators = validatorProvider.getCurrentValidators(); + final Collection
storedValidators = validatorProvider.getValidators(); if (!storedValidators.contains(proposer)) { LOG.trace("Proposer sealing block is not a member of the validators.");