From 6a71c66ad51672da41899c01c1fbf5ed92aec860 Mon Sep 17 00:00:00 2001 From: Trent Mohay <37158202+rain-on@users.noreply.github.com> Date: Wed, 20 Feb 2019 15:09:22 +1100 Subject: [PATCH] IBFT to use VoteTallyCache (#907) It was identified during a demonstration that Pantheon, when running in IBFT would show a "Bad Block Import" when a validator was added or removed from the validator pool. It was determined this was due to IBFT maintaining a single, 'global' copy of the curent list of validators, which was updated when a block was imported - thus when a block which had been imported vi IBFT was then received via Eth block propogation, the validator list would not align with the global list (as it had been updated in the IBFT import). The solution has been to utilise the VoteTallyCache as used in the Clique implementation. Signed-off-by: Adrian Sutton --- .../consensus/clique/CliqueContext.java | 1 + .../consensus/clique/CliqueHelpers.java | 1 + .../blockcreation/CliqueBlockScheduler.java | 2 +- .../blockcreation/CliqueProposerSelector.java | 2 +- .../jsonrpc/CliqueJsonRpcMethodsFactory.java | 5 +- .../jsonrpc/methods/CliqueGetSigners.java | 2 +- .../methods/CliqueGetSignersAtHash.java | 2 +- .../CliqueDifficultyCalculatorTest.java | 1 + .../clique/NodeCanProduceNextBlockTest.java | 1 + .../blockcreation/CliqueBlockCreatorTest.java | 2 +- .../CliqueBlockSchedulerTest.java | 2 +- .../CliqueMinerExecutorTest.java | 2 +- .../CliqueMiningCoordinatorTest.java | 2 +- .../CliqueProposerSelectorTest.java | 2 +- .../CliqueDifficultyValidationRuleTest.java | 2 +- .../CliqueExtraDataValidationRuleTest.java | 2 +- .../methods/CliqueGetSignersAtHashTest.java | 2 +- .../jsonrpc/methods/CliqueGetSignersTest.java | 2 +- .../consensus/common}/VoteTallyCache.java | 17 ++- .../consensus/common}/VoteTallyCacheTest.java | 33 +++--- consensus/ibft/build.gradle | 2 + .../ibft/support/TestContextBuilder.java | 17 ++- .../consensus/ibft/IbftBlockImporter.java | 68 ----------- .../pantheon/consensus/ibft/IbftContext.java | 12 +- .../consensus/ibft/IbftProtocolSchedule.java | 12 +- .../IbftBlockCreatorFactory.java | 11 +- .../ibft/blockcreation/ProposerSelector.java | 71 +++++------- .../IbftCoinbaseValidationRule.java | 5 +- .../IbftCommitSealsValidationRule.java | 5 +- .../IbftValidatorsValidationRule.java | 34 +++--- .../ibft/network/ValidatorPeers.java | 17 +-- .../statemachine/IbftBlockHeightManager.java | 5 +- .../IbftBlockHeightManagerFactory.java | 2 +- .../ibft/statemachine/IbftFinalState.java | 14 +-- .../ibft/statemachine/IbftRoundFactory.java | 2 +- .../validation/MessageValidatorFactory.java | 38 ++++--- .../consensus/ibft/IbftContextBuilder.java | 40 +++++++ .../ibft/IbftProtocolContextFixture.java | 28 ----- ...ockHeaderValidationRulesetFactoryTest.java | 8 +- .../consensus/ibft/IbftBlockImporterTest.java | 107 ------------------ .../blockcreation/IbftBlockCreatorTest.java | 4 +- .../blockcreation/ProposerSelectorTest.java | 49 ++++---- .../IbftCoinbaseValidationRuleTest.java | 8 +- .../IbftCommitSealsValidationRuleTest.java | 19 ++-- .../IbftValidatorsValidationRuleTest.java | 16 ++- .../ibft/network/ValidatorPeersTest.java | 21 ++-- .../IbftBlockHeightManagerTest.java | 9 +- .../ibft/statemachine/IbftRoundTest.java | 7 +- .../ibftlegacy/IbftProtocolSchedule.java | 7 +- .../IbftExtraDataValidationRule.java | 37 +++--- ...ockHeaderValidationRulesetFactoryTest.java | 24 +++- .../blockcreation/IbftBlockCreatorTest.java | 4 +- .../IbftExtraDataValidationRuleTest.java | 26 ++--- .../controller/CliquePantheonController.java | 13 ++- .../IbftLegacyPantheonController.java | 13 ++- .../controller/IbftPantheonController.java | 22 ++-- .../pegasys/pantheon/util/BlockImporter.java | 4 +- 57 files changed, 362 insertions(+), 504 deletions(-) rename consensus/{clique/src/main/java/tech/pegasys/pantheon/consensus/clique => common/src/main/java/tech/pegasys/pantheon/consensus/common}/VoteTallyCache.java (91%) rename consensus/{clique/src/test/java/tech/pegasys/pantheon/consensus/clique => common/src/test/java/tech/pegasys/pantheon/consensus/common}/VoteTallyCacheTest.java (88%) delete mode 100644 consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockImporter.java create mode 100644 consensus/ibft/src/test-support/java/tech/pegasys/pantheon/consensus/ibft/IbftContextBuilder.java delete mode 100644 consensus/ibft/src/test-support/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolContextFixture.java delete mode 100644 consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockImporterTest.java diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueContext.java b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueContext.java index a50d86d625..3449ab95d5 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueContext.java +++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueContext.java @@ -14,6 +14,7 @@ package tech.pegasys.pantheon.consensus.clique; import tech.pegasys.pantheon.consensus.common.EpochManager; import tech.pegasys.pantheon.consensus.common.VoteProposer; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; /** * Holds the data which lives "in parallel" with the importation of blocks etc. when using the 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 a38ed4caad..ee1272588b 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 @@ -15,6 +15,7 @@ package tech.pegasys.pantheon.consensus.clique; import tech.pegasys.pantheon.consensus.clique.blockcreation.CliqueProposerSelector; import tech.pegasys.pantheon.consensus.common.ValidatorProvider; import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.chain.Blockchain; import tech.pegasys.pantheon.ethereum.core.Address; 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 9e63361f5f..5997c95bb0 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 @@ -12,8 +12,8 @@ */ package tech.pegasys.pantheon.consensus.clique.blockcreation; -import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.ValidatorProvider; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.ethereum.blockcreation.DefaultBlockScheduler; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; 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 a8258893a6..6ad85a5040 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,8 +14,8 @@ package tech.pegasys.pantheon.consensus.clique.blockcreation; import static com.google.common.base.Preconditions.checkNotNull; -import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/CliqueJsonRpcMethodsFactory.java b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/CliqueJsonRpcMethodsFactory.java index c344c33b9e..809afa5012 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/CliqueJsonRpcMethodsFactory.java +++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/jsonrpc/CliqueJsonRpcMethodsFactory.java @@ -14,7 +14,6 @@ package tech.pegasys.pantheon.consensus.clique.jsonrpc; import tech.pegasys.pantheon.consensus.clique.CliqueBlockInterface; import tech.pegasys.pantheon.consensus.clique.CliqueContext; -import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.clique.jsonrpc.methods.CliqueGetSigners; import tech.pegasys.pantheon.consensus.clique.jsonrpc.methods.CliqueGetSignersAtHash; import tech.pegasys.pantheon.consensus.clique.jsonrpc.methods.CliqueProposals; @@ -22,6 +21,7 @@ import tech.pegasys.pantheon.consensus.clique.jsonrpc.methods.Discard; import tech.pegasys.pantheon.consensus.clique.jsonrpc.methods.Propose; import tech.pegasys.pantheon.consensus.common.EpochManager; import tech.pegasys.pantheon.consensus.common.VoteProposer; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.VoteTallyUpdater; import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain; @@ -73,6 +73,7 @@ public class CliqueJsonRpcMethodsFactory { final EpochManager epochManager = context.getConsensusState().getEpochManager(); final VoteTallyUpdater voteTallyUpdater = new VoteTallyUpdater(epochManager, new CliqueBlockInterface()); - return new VoteTallyCache(blockchain, voteTallyUpdater, epochManager); + return new VoteTallyCache( + blockchain, voteTallyUpdater, epochManager, new CliqueBlockInterface()); } } 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 260d54d73c..cb3bf21d77 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 @@ -12,7 +12,7 @@ */ package tech.pegasys.pantheon.consensus.clique.jsonrpc.methods; -import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.JsonRpcRequest; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.JsonRpcMethod; 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 c3ea1d2dda..40ab27b6cc 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 @@ -12,7 +12,7 @@ */ package tech.pegasys.pantheon.consensus.clique.jsonrpc.methods; -import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.JsonRpcRequest; 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 01744fc1cb..b22098587b 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 @@ -19,6 +19,7 @@ import static org.mockito.Mockito.when; import tech.pegasys.pantheon.consensus.common.VoteProposer; import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.core.Address; 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 a057a34b58..ac675ce41b 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 @@ -22,6 +22,7 @@ import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.create import tech.pegasys.pantheon.consensus.clique.headervalidationrules.SignerRateLimitValidationRule; import tech.pegasys.pantheon.consensus.common.VoteProposer; import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain; 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 320350f227..6719d85d0b 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 @@ -26,10 +26,10 @@ import tech.pegasys.pantheon.consensus.clique.CliqueExtraData; import tech.pegasys.pantheon.consensus.clique.CliqueHelpers; import tech.pegasys.pantheon.consensus.clique.CliqueProtocolSchedule; import tech.pegasys.pantheon.consensus.clique.TestHelpers; -import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.EpochManager; import tech.pegasys.pantheon.consensus.common.VoteProposer; import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.chain.GenesisState; 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 2f7c593be7..830b356e0d 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 @@ -17,8 +17,8 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.ethereum.blockcreation.AbstractBlockScheduler.BlockCreationTimeResult; import tech.pegasys.pantheon.ethereum.core.Address; 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 fd767332ec..76a130a79c 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 @@ -22,10 +22,10 @@ import tech.pegasys.pantheon.config.GenesisConfigOptions; import tech.pegasys.pantheon.consensus.clique.CliqueContext; import tech.pegasys.pantheon.consensus.clique.CliqueExtraData; import tech.pegasys.pantheon.consensus.clique.CliqueProtocolSchedule; -import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.EpochManager; import tech.pegasys.pantheon.consensus.common.VoteProposer; import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.core.Address; diff --git a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMiningCoordinatorTest.java b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMiningCoordinatorTest.java index 5e0dfe4aba..84789dc86c 100644 --- a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMiningCoordinatorTest.java +++ b/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMiningCoordinatorTest.java @@ -25,8 +25,8 @@ import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.create import tech.pegasys.pantheon.consensus.clique.CliqueContext; import tech.pegasys.pantheon.consensus.clique.CliqueMiningTracker; 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.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain; 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 cfc651b3d8..524a22ec09 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,8 +17,8 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.AddressHelpers; import tech.pegasys.pantheon.ethereum.core.BlockHeaderTestFixture; 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 35ed87d45a..0263e47812 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 @@ -20,9 +20,9 @@ import static org.mockito.Mockito.when; import tech.pegasys.pantheon.consensus.clique.CliqueBlockHashing; import tech.pegasys.pantheon.consensus.clique.CliqueContext; import tech.pegasys.pantheon.consensus.clique.CliqueExtraData; -import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.VoteProposer; import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.crypto.SECP256K1.Signature; 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 cb323495a0..d6a91d2cd8 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 @@ -20,9 +20,9 @@ import static org.mockito.Mockito.when; import tech.pegasys.pantheon.consensus.clique.CliqueContext; import tech.pegasys.pantheon.consensus.clique.CliqueExtraData; import tech.pegasys.pantheon.consensus.clique.TestHelpers; -import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.EpochManager; import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.core.Address; 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 dd173a5334..e673011992 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 @@ -19,8 +19,8 @@ import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.when; import static tech.pegasys.pantheon.ethereum.core.Address.fromHexString; -import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.core.BlockHeaderTestFixture; 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 0e97e6603c..ff19bef53d 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 @@ -19,8 +19,8 @@ import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.when; import static tech.pegasys.pantheon.ethereum.core.Address.fromHexString; -import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.core.BlockHeaderTestFixture; diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/VoteTallyCache.java b/consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/VoteTallyCache.java similarity index 91% rename from consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/VoteTallyCache.java rename to consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/VoteTallyCache.java index 4783e7b350..472e7b1835 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/VoteTallyCache.java +++ b/consensus/common/src/main/java/tech/pegasys/pantheon/consensus/common/VoteTallyCache.java @@ -10,13 +10,10 @@ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the * specific language governing permissions and limitations under the License. */ -package tech.pegasys.pantheon.consensus.clique; +package tech.pegasys.pantheon.consensus.common; import static com.google.common.base.Preconditions.checkNotNull; -import tech.pegasys.pantheon.consensus.common.EpochManager; -import tech.pegasys.pantheon.consensus.common.VoteTally; -import tech.pegasys.pantheon.consensus.common.VoteTallyUpdater; import tech.pegasys.pantheon.ethereum.chain.Blockchain; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.core.Hash; @@ -37,17 +34,26 @@ public class VoteTallyCache { private final Cache voteTallyCache = CacheBuilder.newBuilder().maximumSize(100).build(); + private BlockInterface blockInterface; public VoteTallyCache( final Blockchain blockchain, final VoteTallyUpdater voteTallyUpdater, - final EpochManager epochManager) { + final EpochManager epochManager, + final BlockInterface blockInterface) { + checkNotNull(blockchain); checkNotNull(voteTallyUpdater); checkNotNull(epochManager); + checkNotNull(blockInterface); this.blockchain = blockchain; this.voteTallyUpdater = voteTallyUpdater; this.epochManager = epochManager; + this.blockInterface = blockInterface; + } + + public VoteTally getVoteTallyAtHead() { + return getVoteTallyAfterBlock(blockchain.getChainHeadHeader()); } /** @@ -94,7 +100,6 @@ public class VoteTallyCache { private VoteTally getValidatorsAfter(final BlockHeader header) { if (epochManager.isEpochBlock(header.getNumber())) { - final CliqueBlockInterface blockInterface = new CliqueBlockInterface(); return new VoteTally(blockInterface.validatorsInBlock(header)); } diff --git a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/VoteTallyCacheTest.java b/consensus/common/src/test/java/tech/pegasys/pantheon/consensus/common/VoteTallyCacheTest.java similarity index 88% rename from consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/VoteTallyCacheTest.java rename to consensus/common/src/test/java/tech/pegasys/pantheon/consensus/common/VoteTallyCacheTest.java index 954a07548d..4193ab8754 100644 --- a/consensus/clique/src/test/java/tech/pegasys/pantheon/consensus/clique/VoteTallyCacheTest.java +++ b/consensus/common/src/test/java/tech/pegasys/pantheon/consensus/common/VoteTallyCacheTest.java @@ -10,7 +10,7 @@ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the * specific language governing permissions and limitations under the License. */ -package tech.pegasys.pantheon.consensus.clique; +package tech.pegasys.pantheon.consensus.common; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -24,11 +24,6 @@ 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; @@ -39,7 +34,6 @@ import tech.pegasys.pantheon.ethereum.core.BlockHeaderTestFixture; import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.util.bytes.BytesValue; -import java.math.BigInteger; import java.util.Arrays; import java.util.List; import java.util.Optional; @@ -52,7 +46,7 @@ import org.mockito.ArgumentCaptor; public class VoteTallyCacheTest { - BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture(); + private final BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture(); private Block createEmptyBlock(final long blockNumber, final Hash parentHash) { headerBuilder.number(blockNumber).parentHash(parentHash).coinbase(AddressHelpers.ofValue(0)); @@ -60,24 +54,21 @@ public class VoteTallyCacheTest { headerBuilder.buildHeader(), new BlockBody(Lists.emptyList(), Lists.emptyList())); } - MutableBlockchain blockChain; + private MutableBlockchain blockChain; private Block genesisBlock; private Block block_1; private Block block_2; private final List
validators = Lists.newArrayList(); + private final BlockInterface blockInterface = mock(BlockInterface.class); + @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), - validators) - .encode()); + headerBuilder.extraData(BytesValue.wrap(new byte[32])); genesisBlock = createEmptyBlock(0, Hash.ZERO); @@ -88,13 +79,15 @@ public class VoteTallyCacheTest { blockChain.appendBlock(block_1, Lists.emptyList()); blockChain.appendBlock(block_2, Lists.emptyList()); + + when(blockInterface.validatorsInBlock(any())).thenReturn(validators); } @Test public void parentBlockVoteTallysAreCachedWhenChildVoteTallyRequested() { final VoteTallyUpdater tallyUpdater = mock(VoteTallyUpdater.class); final VoteTallyCache cache = - new VoteTallyCache(blockChain, tallyUpdater, new EpochManager(30_000)); + new VoteTallyCache(blockChain, tallyUpdater, new EpochManager(30_000), blockInterface); // The votetallyUpdater should be invoked for the requested block, and all parents including // the epoch (genesis) block. @@ -120,7 +113,7 @@ public class VoteTallyCacheTest { public void exceptionThrownIfNoParentBlockExists() { final VoteTallyUpdater tallyUpdater = mock(VoteTallyUpdater.class); final VoteTallyCache cache = - new VoteTallyCache(blockChain, tallyUpdater, new EpochManager(30_000)); + new VoteTallyCache(blockChain, tallyUpdater, new EpochManager(30_000), blockInterface); final Block orphanBlock = createEmptyBlock(4, Hash.ZERO); @@ -134,7 +127,7 @@ public class VoteTallyCacheTest { public void walkBackStopsWhenACachedVoteTallyIsFound() { final VoteTallyUpdater tallyUpdater = mock(VoteTallyUpdater.class); final VoteTallyCache cache = - new VoteTallyCache(blockChain, tallyUpdater, new EpochManager(30_000)); + new VoteTallyCache(blockChain, tallyUpdater, new EpochManager(30_000), blockInterface); // Load the Cache up to block_2 cache.getVoteTallyAfterBlock(block_2.getHeader()); @@ -159,7 +152,6 @@ public class VoteTallyCacheTest { @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())) @@ -168,7 +160,8 @@ public class VoteTallyCacheTest { 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); + final VoteTallyCache cache = + new VoteTallyCache(blockChain, tallyUpdater, epochManager, blockInterface); VoteTally voteTally = cache.getVoteTallyAfterBlock(block_1.getHeader()); assertThat(voteTally.getValidators()).containsAll(validators); diff --git a/consensus/ibft/build.gradle b/consensus/ibft/build.gradle index 3e800c927a..829fd80fd0 100644 --- a/consensus/ibft/build.gradle +++ b/consensus/ibft/build.gradle @@ -55,6 +55,8 @@ dependencies { testImplementation 'org.awaitility:awaitility' testImplementation 'org.assertj:assertj-core' testImplementation 'org.mockito:mockito-core' + + testSupportImplementation 'org.mockito:mockito-core' } diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextBuilder.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextBuilder.java index e7083af57b..3e46fe78dc 100644 --- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextBuilder.java +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextBuilder.java @@ -21,7 +21,7 @@ import tech.pegasys.pantheon.config.StubGenesisConfigOptions; import tech.pegasys.pantheon.consensus.common.BlockInterface; import tech.pegasys.pantheon.consensus.common.EpochManager; import tech.pegasys.pantheon.consensus.common.VoteProposer; -import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.VoteTallyUpdater; import tech.pegasys.pantheon.consensus.ibft.BlockTimer; import tech.pegasys.pantheon.consensus.ibft.EventMultiplexer; @@ -249,14 +249,19 @@ public class TestContextBuilder { final EpochManager epochManager = new EpochManager(EPOCH_LENGTH); final BlockInterface blockInterface = new IbftBlockInterface(); - final VoteTally voteTally = - new VoteTallyUpdater(epochManager, blockInterface).buildVoteTallyFromBlockchain(blockChain); + + final VoteTallyCache voteTallyCache = + new VoteTallyCache( + blockChain, + new VoteTallyUpdater(epochManager, blockInterface), + epochManager, + new IbftBlockInterface()); final VoteProposer voteProposer = new VoteProposer(); final ProtocolContext protocolContext = new ProtocolContext<>( - blockChain, worldStateArchive, new IbftContext(voteTally, voteProposer)); + blockChain, worldStateArchive, new IbftContext(voteTallyCache, voteProposer)); final IbftBlockCreatorFactory blockCreatorFactory = new IbftBlockCreatorFactory( @@ -268,11 +273,11 @@ public class TestContextBuilder { Util.publicKeyToAddress(nodeKeys.getPublicKey())); final ProposerSelector proposerSelector = - new ProposerSelector(blockChain, voteTally, blockInterface, true); + new ProposerSelector(blockChain, blockInterface, true); final IbftFinalState finalState = new IbftFinalState( - voteTally, + protocolContext.getConsensusState().getVoteTallyCache(), nodeKeys, Util.publicKeyToAddress(nodeKeys.getPublicKey()), proposerSelector, diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockImporter.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockImporter.java deleted file mode 100644 index 125d721d30..0000000000 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockImporter.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright 2018 ConsenSys AG. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ -package tech.pegasys.pantheon.consensus.ibft; - -import tech.pegasys.pantheon.consensus.common.VoteTallyUpdater; -import tech.pegasys.pantheon.ethereum.ProtocolContext; -import tech.pegasys.pantheon.ethereum.core.Block; -import tech.pegasys.pantheon.ethereum.core.BlockHeader; -import tech.pegasys.pantheon.ethereum.core.BlockImporter; -import tech.pegasys.pantheon.ethereum.core.TransactionReceipt; -import tech.pegasys.pantheon.ethereum.mainnet.HeaderValidationMode; - -import java.util.List; - -/** - * The IBFT BlockImporter implementation. Adds votes to VoteTally as blocks are added to the chain. - */ -public class IbftBlockImporter implements BlockImporter { - - private final BlockImporter delegate; - private final VoteTallyUpdater voteTallyUpdater; - - public IbftBlockImporter( - final BlockImporter delegate, final VoteTallyUpdater voteTallyUpdater) { - this.delegate = delegate; - this.voteTallyUpdater = voteTallyUpdater; - } - - @Override - public boolean importBlock( - final ProtocolContext context, - final Block block, - final HeaderValidationMode headerValidationMode, - final HeaderValidationMode ommerValidationMode) { - final boolean result = - delegate.importBlock(context, block, headerValidationMode, ommerValidationMode); - updateVoteTally(result, block.getHeader(), context); - return result; - } - - @Override - public boolean fastImportBlock( - final ProtocolContext context, - final Block block, - final List receipts, - final HeaderValidationMode headerValidationMode) { - final boolean result = delegate.fastImportBlock(context, block, receipts, headerValidationMode); - updateVoteTally(result, block.getHeader(), context); - return result; - } - - private void updateVoteTally( - final boolean result, final BlockHeader header, final ProtocolContext context) { - if (result) { - voteTallyUpdater.updateForBlock(header, context.getConsensusState().getVoteTally()); - } - } -} diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftContext.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftContext.java index 86b2e65012..dd1db10b65 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftContext.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftContext.java @@ -13,21 +13,21 @@ package tech.pegasys.pantheon.consensus.ibft; import tech.pegasys.pantheon.consensus.common.VoteProposer; -import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; /** Holds the IBFT specific mutable state. */ public class IbftContext { - private final VoteTally voteTally; + private final VoteTallyCache voteTallyCache; private final VoteProposer voteProposer; - public IbftContext(final VoteTally voteTally, final VoteProposer voteProposer) { - this.voteTally = voteTally; + public IbftContext(final VoteTallyCache voteTallyCache, final VoteProposer voteProposer) { + this.voteTallyCache = voteTallyCache; this.voteProposer = voteProposer; } - public VoteTally getVoteTally() { - return voteTally; + public VoteTallyCache getVoteTallyCache() { + return voteTallyCache; } public VoteProposer getVoteProposer() { diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolSchedule.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolSchedule.java index 014028980e..9e18bdde70 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolSchedule.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolSchedule.java @@ -17,7 +17,6 @@ import static tech.pegasys.pantheon.consensus.ibft.IbftBlockHeaderValidationRule import tech.pegasys.pantheon.config.GenesisConfigOptions; import tech.pegasys.pantheon.config.IbftConfigOptions; import tech.pegasys.pantheon.consensus.common.EpochManager; -import tech.pegasys.pantheon.consensus.common.VoteTallyUpdater; import tech.pegasys.pantheon.ethereum.MainnetBlockValidator; import tech.pegasys.pantheon.ethereum.core.PrivacyParameters; import tech.pegasys.pantheon.ethereum.core.Wei; @@ -43,25 +42,20 @@ public class IbftProtocolSchedule { return new ProtocolScheduleBuilder<>( config, DEFAULT_CHAIN_ID, - builder -> applyIbftChanges(blockPeriod, epochManager, builder), + builder -> applyIbftChanges(blockPeriod, builder), PrivacyParameters.noPrivacy()) .createProtocolSchedule(); } private static ProtocolSpecBuilder applyIbftChanges( - final long secondsBetweenBlocks, - final EpochManager epochManager, - final ProtocolSpecBuilder builder) { + final long secondsBetweenBlocks, final ProtocolSpecBuilder builder) { return builder .changeConsensusContextType( difficultyCalculator -> ibftBlockHeaderValidator(secondsBetweenBlocks), difficultyCalculator -> ibftBlockHeaderValidator(secondsBetweenBlocks), MainnetBlockBodyValidator::new, MainnetBlockValidator::new, - (blockValidator) -> - new IbftBlockImporter( - new MainnetBlockImporter<>(blockValidator), - new VoteTallyUpdater(epochManager, new IbftBlockInterface())), + MainnetBlockImporter::new, (time, parent, protocolContext) -> BigInteger.ONE) .blockReward(Wei.ZERO) .blockHashFunction(IbftBlockHashing::calculateHashOfIbftBlockOnChain); 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 1ccb453790..e31768d4a8 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 @@ -63,7 +63,7 @@ public class IbftBlockCreatorFactory { public IbftBlockCreator create(final BlockHeader parentHeader, final int round) { return new IbftBlockCreator( localAddress, - ph -> createExtraData(round), + ph -> createExtraData(round, ph), pendingTransactions, protocolContext, protocolSchedule, @@ -84,8 +84,13 @@ public class IbftBlockCreatorFactory { return minTransactionGasPrice; } - public BytesValue createExtraData(final int round) { - final VoteTally voteTally = protocolContext.getConsensusState().getVoteTally(); + public BytesValue createExtraData(final int round, final BlockHeader parentHeader) { + final VoteTally voteTally = + protocolContext + .getConsensusState() + .getVoteTallyCache() + .getVoteTallyAfterBlock(parentHeader); + final Optional proposal = protocolContext.getConsensusState().getVoteProposer().getVote(localAddress, voteTally); 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 98b96dc483..5e1b4a3ab0 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 @@ -22,6 +22,7 @@ import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.NavigableSet; import java.util.Optional; @@ -46,9 +47,6 @@ public class ProposerSelector { private final Blockchain blockchain; - /** Provides the current list of validators */ - private final ValidatorProvider validators; - /** * If set, will cause the proposer to change on successful addition of a block. Otherwise, the * previously successful proposer will propose the next block as well. @@ -59,11 +57,9 @@ public class ProposerSelector { public ProposerSelector( final Blockchain blockchain, - final ValidatorProvider validators, final BlockInterface blockInterface, final boolean changeEachBlock) { this.blockchain = blockchain; - this.validators = validators; this.blockInterface = blockInterface; this.changeEachBlock = changeEachBlock; } @@ -75,17 +71,24 @@ public class ProposerSelector { * @return The address of the node which is to propose a block for the provided Round. */ public Address selectProposerForRound(final ConsensusRoundIdentifier roundIdentifier) { - checkArgument(roundIdentifier.getRoundNumber() >= 0); checkArgument(roundIdentifier.getSequenceNumber() > 0); final long prevBlockNumber = roundIdentifier.getSequenceNumber() - 1; - final Address prevBlockProposer = getProposerOfBlock(prevBlockNumber); + final Optional maybeParentHeader = blockchain.getBlockHeader(prevBlockNumber); + if (!maybeParentHeader.isPresent()) { + LOG.trace("Unable to determine proposer for requested block"); + throw new RuntimeException("Unable to determine past proposer"); + } - if (!validators.getValidators().contains(prevBlockProposer)) { - return handleMissingProposer(prevBlockProposer, roundIdentifier); + final BlockHeader blockHeader = maybeParentHeader.get(); + final Address prevBlockProposer = blockInterface.getProposerOfBlock(blockHeader); + final Collection
validatorsForRound = blockInterface.validatorsInBlock(blockHeader); + + if (!validatorsForRound.contains(prevBlockProposer)) { + return handleMissingProposer(prevBlockProposer, validatorsForRound, roundIdentifier); } else { - return handleWithExistingProposer(prevBlockProposer, roundIdentifier); + return handleWithExistingProposer(prevBlockProposer, validatorsForRound, roundIdentifier); } } @@ -96,8 +99,10 @@ public class ProposerSelector { *

And validators will change from there. */ private Address handleMissingProposer( - final Address prevBlockProposer, final ConsensusRoundIdentifier roundIdentifier) { - final NavigableSet

validatorSet = new TreeSet<>(validators.getValidators()); + final Address prevBlockProposer, + final Collection
validatorsForRound, + final ConsensusRoundIdentifier roundIdentifier) { + final NavigableSet
validatorSet = new TreeSet<>(validatorsForRound); final SortedSet
latterValidators = validatorSet.tailSet(prevBlockProposer, false); final Address nextProposer; if (latterValidators.isEmpty()) { @@ -108,57 +113,37 @@ public class ProposerSelector { // Else, use the first validator after the dropped entry. nextProposer = latterValidators.first(); } - return calculateRoundSpecificValidator(nextProposer, roundIdentifier.getRoundNumber()); + return calculateRoundSpecificValidator( + nextProposer, validatorsForRound, roundIdentifier.getRoundNumber()); } /** * If the previous Proposer is still a validator - determine what offset should be applied for the * given round - factoring in a proposer change on the new block. - * - * @param prevBlockProposer - * @param roundIdentifier - * @return */ private Address handleWithExistingProposer( - final Address prevBlockProposer, final ConsensusRoundIdentifier roundIdentifier) { + final Address prevBlockProposer, + final Collection
validatorsForRound, + final ConsensusRoundIdentifier roundIdentifier) { int indexOffsetFromPrevBlock = roundIdentifier.getRoundNumber(); if (changeEachBlock) { indexOffsetFromPrevBlock += 1; } - return calculateRoundSpecificValidator(prevBlockProposer, indexOffsetFromPrevBlock); + return calculateRoundSpecificValidator( + prevBlockProposer, validatorsForRound, indexOffsetFromPrevBlock); } /** * Given Round 0 of the given height should start from given proposer (baseProposer) - determine * which validator should be used given the indexOffset. - * - * @param baseProposer - * @param indexOffset - * @return */ private Address calculateRoundSpecificValidator( - final Address baseProposer, final int indexOffset) { - final List
currentValidatorList = new ArrayList<>(validators.getValidators()); + final Address baseProposer, + final Collection
validatorsForRound, + final int indexOffset) { + final List
currentValidatorList = new ArrayList<>(validatorsForRound); final int prevProposerIndex = currentValidatorList.indexOf(baseProposer); final int roundValidatorIndex = (prevProposerIndex + indexOffset) % currentValidatorList.size(); return currentValidatorList.get(roundValidatorIndex); } - - /** - * Determines the proposer of an existing block, based on the proposer signature in the extra - * data. - * - * @param blockNumber The index of the block in the chain being queried. - * @return The unique identifier fo the node which proposed the block number in question. - */ - private Address getProposerOfBlock(final long blockNumber) { - final Optional maybeBlockHeader = blockchain.getBlockHeader(blockNumber); - if (maybeBlockHeader.isPresent()) { - final BlockHeader blockHeader = maybeBlockHeader.get(); - return blockInterface.getProposerOfBlock(blockHeader); - } else { - LOG.trace("Unable to determine proposer for requested block"); - throw new RuntimeException("Unable to determine past proposer"); - } - } } 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 733398f7f8..7281106e3c 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 @@ -38,8 +38,9 @@ public class IbftCoinbaseValidationRule implements AttachedBlockHeaderValidation final BlockHeader parent, final ProtocolContext context) { - final ValidatorProvider validatorProvider = context.getConsensusState().getVoteTally(); - Address proposer = header.getCoinbase(); + final ValidatorProvider validatorProvider = + context.getConsensusState().getVoteTallyCache().getVoteTallyAfterBlock(parent); + final Address proposer = header.getCoinbase(); final Collection
storedValidators = validatorProvider.getValidators(); diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCommitSealsValidationRule.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCommitSealsValidationRule.java index 41dfa05eef..f81ae8438f 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCommitSealsValidationRule.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCommitSealsValidationRule.java @@ -47,12 +47,13 @@ public class IbftCommitSealsValidationRule final BlockHeader header, final BlockHeader parent, final ProtocolContext protocolContext) { - final ValidatorProvider validatorProvider = protocolContext.getConsensusState().getVoteTally(); + final ValidatorProvider validatorProvider = + protocolContext.getConsensusState().getVoteTallyCache().getVoteTallyAfterBlock(parent); final IbftExtraData ibftExtraData = IbftExtraData.decode(header.getExtraData()); final List
committers = IbftBlockHashing.recoverCommitterAddresses(header, ibftExtraData); - List
committersWithoutDuplicates = new ArrayList<>(new HashSet<>(committers)); + final List
committersWithoutDuplicates = new ArrayList<>(new HashSet<>(committers)); if (committers.size() != committersWithoutDuplicates.size()) { LOGGER.trace("Duplicated seals found in header."); diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftValidatorsValidationRule.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftValidatorsValidationRule.java index 8ffdca71aa..1a5a8f0215 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftValidatorsValidationRule.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftValidatorsValidationRule.java @@ -22,6 +22,8 @@ import tech.pegasys.pantheon.ethereum.mainnet.AttachedBlockHeaderValidationRule; import tech.pegasys.pantheon.ethereum.rlp.RLPException; import java.util.Collection; +import java.util.SortedSet; +import java.util.TreeSet; import com.google.common.collect.Iterables; import org.apache.logging.log4j.LogManager; @@ -41,29 +43,23 @@ public class IbftValidatorsValidationRule final BlockHeader header, final BlockHeader parent, final ProtocolContext context) { - return validateExtraData(header, context); - } - - /** - * Responsible for determining the validity of the extra data field. Ensures: - * - *
    - *
  • Bytes in the extra data field can be decoded as per IBFT specification - *
  • Validators in block matches that tracked in memory. - *
- * - * @param header the block header containing the extraData to be validated. - * @return True if the extraData successfully produces an IstanbulExtraData object, false - * otherwise - */ - private boolean validateExtraData( - final BlockHeader header, final ProtocolContext context) { try { - final ValidatorProvider validatorProvider = context.getConsensusState().getVoteTally(); + final ValidatorProvider validatorProvider = + context.getConsensusState().getVoteTallyCache().getVoteTallyAfterBlock(parent); final IbftExtraData ibftExtraData = IbftExtraData.decode(header.getExtraData()); - final Collection
storedValidators = validatorProvider.getValidators(); + final SortedSet
sortedReportedValidators = + new TreeSet<>(ibftExtraData.getValidators()); + if (!Iterables.elementsEqual(ibftExtraData.getValidators(), sortedReportedValidators)) { + LOGGER.trace( + "Validators are not sorted in ascending order. Expected {} but got {}.", + sortedReportedValidators, + ibftExtraData.getValidators()); + return false; + } + + final Collection
storedValidators = validatorProvider.getValidators(); if (!Iterables.elementsEqual(ibftExtraData.getValidators(), storedValidators)) { LOGGER.trace( "Incorrect validators. Expected {} but got {}.", diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/ValidatorPeers.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/ValidatorPeers.java index ae90e3cd49..112ab62919 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/ValidatorPeers.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/ValidatorPeers.java @@ -12,7 +12,7 @@ */ package tech.pegasys.pantheon.consensus.ibft.network; -import tech.pegasys.pantheon.consensus.common.ValidatorProvider; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; @@ -38,10 +38,10 @@ public class ValidatorPeers implements ValidatorMulticaster, PeerConnectionTrack private static final String PROTOCOL_NAME = "IBF"; private final Map peerConnections = Maps.newConcurrentMap(); - private final ValidatorProvider validatorProvider; + private final VoteTallyCache voteTallyCache; - public ValidatorPeers(final ValidatorProvider validatorProvider) { - this.validatorProvider = validatorProvider; + public ValidatorPeers(final VoteTallyCache voteTallyCache) { + this.voteTallyCache = voteTallyCache; } @Override @@ -58,14 +58,13 @@ public class ValidatorPeers implements ValidatorMulticaster, PeerConnectionTrack @Override public void send(final MessageData message) { - final Collection
validators = validatorProvider.getValidators(); - sendMessageToSpecificAddresses(validators, message); + sendMessageToSpecificAddresses(getLatestValidators(), message); } @Override public void send(final MessageData message, final Collection
blackList) { final Collection
includedValidators = - validatorProvider.getValidators().stream() + getLatestValidators().stream() .filter(a -> !blackList.contains(a)) .collect(Collectors.toSet()); sendMessageToSpecificAddresses(includedValidators, message); @@ -90,4 +89,8 @@ public class ValidatorPeers implements ValidatorMulticaster, PeerConnectionTrack } }); } + + private Collection
getLatestValidators() { + return voteTallyCache.getVoteTallyAtHead().getValidators(); + } } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java index 9b4be95fbf..061dfcb104 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java @@ -90,14 +90,15 @@ public class IbftBlockHeightManager implements BlockHeightManager { this.roundChangeManager = roundChangeManager; this.finalState = finalState; - newRoundMessageValidator = messageValidatorFactory.createNewRoundValidator(getChainHeight()); + newRoundMessageValidator = + messageValidatorFactory.createNewRoundValidator(getChainHeight(), parentHeader); roundStateCreator = (roundIdentifier) -> new RoundState( roundIdentifier, finalState.getQuorum(), - messageValidatorFactory.createMessageValidator(roundIdentifier)); + messageValidatorFactory.createMessageValidator(roundIdentifier, parentHeader)); } @Override diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerFactory.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerFactory.java index c3d9d892ab..a71afd6ec0 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerFactory.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerFactory.java @@ -50,7 +50,7 @@ public class IbftBlockHeightManagerFactory { new RoundChangeManager( IbftHelpers.calculateRequiredValidatorQuorum(finalState.getValidators().size()), messageValidatorFactory.createRoundChangeMessageValidator( - parentHeader.getNumber() + 1L)), + parentHeader.getNumber() + 1L, parentHeader)), roundFactory, finalState.getClock(), messageValidatorFactory); diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftFinalState.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftFinalState.java index e5ef69dda5..4a9e4af3af 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftFinalState.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftFinalState.java @@ -14,7 +14,7 @@ package tech.pegasys.pantheon.consensus.ibft.statemachine; import static tech.pegasys.pantheon.consensus.ibft.IbftHelpers.calculateRequiredValidatorQuorum; -import tech.pegasys.pantheon.consensus.common.ValidatorProvider; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.consensus.ibft.BlockTimer; import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; import tech.pegasys.pantheon.consensus.ibft.RoundTimer; @@ -31,11 +31,10 @@ import java.util.Collection; /** This is the full data set, or context, required for many of the aspects of the IBFT workflow. */ public class IbftFinalState { - private final ValidatorProvider validatorProvider; + private final VoteTallyCache voteTallyCache; private final KeyPair nodeKeys; private final Address localAddress; private final ProposerSelector proposerSelector; - private final ValidatorMulticaster validatorMulticaster; private final RoundTimer roundTimer; private final BlockTimer blockTimer; private final IbftBlockCreatorFactory blockCreatorFactory; @@ -44,7 +43,7 @@ public class IbftFinalState { private final Clock clock; public IbftFinalState( - final ValidatorProvider validatorProvider, + final VoteTallyCache voteTallyCache, final KeyPair nodeKeys, final Address localAddress, final ProposerSelector proposerSelector, @@ -54,11 +53,10 @@ public class IbftFinalState { final IbftBlockCreatorFactory blockCreatorFactory, final MessageFactory messageFactory, final Clock clock) { - this.validatorProvider = validatorProvider; + this.voteTallyCache = voteTallyCache; this.nodeKeys = nodeKeys; this.localAddress = localAddress; this.proposerSelector = proposerSelector; - this.validatorMulticaster = validatorMulticaster; this.roundTimer = roundTimer; this.blockTimer = blockTimer; this.blockCreatorFactory = blockCreatorFactory; @@ -68,11 +66,11 @@ public class IbftFinalState { } public int getQuorum() { - return calculateRequiredValidatorQuorum(validatorProvider.getValidators().size()); + return calculateRequiredValidatorQuorum(getValidators().size()); } public Collection
getValidators() { - return validatorProvider.getValidators(); + return voteTallyCache.getVoteTallyAtHead().getValidators(); } public KeyPair getNodeKeys() { diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundFactory.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundFactory.java index b99a7e8e2f..d6dad5fdef 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundFactory.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundFactory.java @@ -54,7 +54,7 @@ public class IbftRoundFactory { new RoundState( roundIdentifier, finalState.getQuorum(), - messageValidatorFactory.createMessageValidator(roundIdentifier)); + messageValidatorFactory.createMessageValidator(roundIdentifier, parentHeader)); return createNewRoundWithState(parentHeader, roundState); } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/MessageValidatorFactory.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/MessageValidatorFactory.java index 1e41c038ea..469e714e98 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/MessageValidatorFactory.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/MessageValidatorFactory.java @@ -21,6 +21,7 @@ import tech.pegasys.pantheon.consensus.ibft.blockcreation.ProposerSelector; import tech.pegasys.pantheon.ethereum.BlockValidator; import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.core.Address; +import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule; import java.util.Collection; @@ -40,33 +41,42 @@ public class MessageValidatorFactory { this.protocolContext = protocolContext; } + private Collection
getValidatorsAfterBlock(final BlockHeader parentHeader) { + return protocolContext + .getConsensusState() + .getVoteTallyCache() + .getVoteTallyAfterBlock(parentHeader) + .getValidators(); + } + private SignedDataValidator createSignedDataValidator( - final ConsensusRoundIdentifier roundIdentifier) { + final ConsensusRoundIdentifier roundIdentifier, final BlockHeader parentHeader) { return new SignedDataValidator( - protocolContext.getConsensusState().getVoteTally().getValidators(), + getValidatorsAfterBlock(parentHeader), proposerSelector.selectProposerForRound(roundIdentifier), roundIdentifier); } - public MessageValidator createMessageValidator(final ConsensusRoundIdentifier roundIdentifier) { + public MessageValidator createMessageValidator( + final ConsensusRoundIdentifier roundIdentifier, final BlockHeader parentHeader) { final BlockValidator blockValidator = protocolSchedule.getByBlockNumber(roundIdentifier.getSequenceNumber()).getBlockValidator(); return new MessageValidator( - createSignedDataValidator(roundIdentifier), + createSignedDataValidator(roundIdentifier, parentHeader), new ProposalBlockConsistencyValidator(), blockValidator, protocolContext); } - public RoundChangeMessageValidator createRoundChangeMessageValidator(final long chainHeight) { - final Collection
validators = - protocolContext.getConsensusState().getVoteTally().getValidators(); + public RoundChangeMessageValidator createRoundChangeMessageValidator( + final long chainHeight, final BlockHeader parentHeader) { + final Collection
validators = getValidatorsAfterBlock(parentHeader); return new RoundChangeMessageValidator( new RoundChangePayloadValidator( - this::createSignedDataValidator, + (roundIdentifier) -> createSignedDataValidator(roundIdentifier, parentHeader), validators, prepareMessageCountForQuorum( IbftHelpers.calculateRequiredValidatorQuorum(validators.size())), @@ -74,21 +84,23 @@ public class MessageValidatorFactory { new ProposalBlockConsistencyValidator()); } - public NewRoundMessageValidator createNewRoundValidator(final long chainHeight) { - final Collection
validators = - protocolContext.getConsensusState().getVoteTally().getValidators(); + public NewRoundMessageValidator createNewRoundValidator( + final long chainHeight, final BlockHeader parentHeader) { + final Collection
validators = getValidatorsAfterBlock(parentHeader); final BlockValidator blockValidator = protocolSchedule.getByBlockNumber(chainHeight).getBlockValidator(); final RoundChangeCertificateValidator roundChangeCertificateValidator = new RoundChangeCertificateValidator( - validators, this::createSignedDataValidator, chainHeight); + validators, + (roundIdentifier) -> createSignedDataValidator(roundIdentifier, parentHeader), + chainHeight); return new NewRoundMessageValidator( new NewRoundPayloadValidator( proposerSelector, - this::createSignedDataValidator, + (roundIdentifier) -> createSignedDataValidator(roundIdentifier, parentHeader), chainHeight, roundChangeCertificateValidator), new ProposalBlockConsistencyValidator(), diff --git a/consensus/ibft/src/test-support/java/tech/pegasys/pantheon/consensus/ibft/IbftContextBuilder.java b/consensus/ibft/src/test-support/java/tech/pegasys/pantheon/consensus/ibft/IbftContextBuilder.java new file mode 100644 index 0000000000..e9c0aea359 --- /dev/null +++ b/consensus/ibft/src/test-support/java/tech/pegasys/pantheon/consensus/ibft/IbftContextBuilder.java @@ -0,0 +1,40 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.consensus.ibft; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; + +import tech.pegasys.pantheon.consensus.common.VoteProposer; +import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; +import tech.pegasys.pantheon.ethereum.core.Address; + +import java.util.Collection; + +public class IbftContextBuilder { + + public static IbftContext setupContextWithValidators(final Collection
validators) { + final IbftContext ibftContext = mock(IbftContext.class, withSettings().lenient()); + final VoteTallyCache mockCache = mock(VoteTallyCache.class, withSettings().lenient()); + final VoteTally mockVoteTally = mock(VoteTally.class, withSettings().lenient()); + when(ibftContext.getVoteTallyCache()).thenReturn(mockCache); + when(mockCache.getVoteTallyAfterBlock(any())).thenReturn(mockVoteTally); + when(mockVoteTally.getValidators()).thenReturn(validators); + when(ibftContext.getVoteProposer()).thenReturn(new VoteProposer()); + + return ibftContext; + } +} diff --git a/consensus/ibft/src/test-support/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolContextFixture.java b/consensus/ibft/src/test-support/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolContextFixture.java deleted file mode 100644 index c4dd2932f1..0000000000 --- a/consensus/ibft/src/test-support/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolContextFixture.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright 2018 ConsenSys AG. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ -package tech.pegasys.pantheon.consensus.ibft; - -import tech.pegasys.pantheon.consensus.common.VoteProposer; -import tech.pegasys.pantheon.consensus.common.VoteTally; -import tech.pegasys.pantheon.ethereum.ProtocolContext; -import tech.pegasys.pantheon.ethereum.core.Address; - -import java.util.List; - -public class IbftProtocolContextFixture { - - public static ProtocolContext protocolContext(final List
validators) { - return new ProtocolContext<>( - null, null, new IbftContext(new VoteTally(validators), new VoteProposer())); - } -} diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockHeaderValidationRulesetFactoryTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockHeaderValidationRulesetFactoryTest.java index 99c7c07241..80649af434 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockHeaderValidationRulesetFactoryTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockHeaderValidationRulesetFactoryTest.java @@ -15,9 +15,10 @@ package tech.pegasys.pantheon.consensus.ibft; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; -import static tech.pegasys.pantheon.consensus.ibft.IbftProtocolContextFixture.protocolContext; +import static tech.pegasys.pantheon.consensus.ibft.IbftContextBuilder.setupContextWithValidators; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; +import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.core.BlockHeaderTestFixture; @@ -28,6 +29,7 @@ import tech.pegasys.pantheon.ethereum.mainnet.HeaderValidationMode; import tech.pegasys.pantheon.util.bytes.BytesValue; import tech.pegasys.pantheon.util.uint.UInt256; +import java.util.Collection; import java.util.List; import java.util.Optional; @@ -35,6 +37,10 @@ import org.junit.Test; public class IbftBlockHeaderValidationRulesetFactoryTest { + private final ProtocolContext protocolContext(final Collection
validators) { + return new ProtocolContext<>(null, null, setupContextWithValidators(validators)); + } + @Test public void ibftValidateHeaderPasses() { final KeyPair proposerKeyPair = KeyPair.generate(); diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockImporterTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockImporterTest.java deleted file mode 100644 index dd219aaa62..0000000000 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockImporterTest.java +++ /dev/null @@ -1,107 +0,0 @@ -/* - * Copyright 2018 ConsenSys AG. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ -package tech.pegasys.pantheon.consensus.ibft; - -import static java.util.Collections.emptyList; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; -import static org.mockito.Mockito.when; - -import tech.pegasys.pantheon.consensus.common.VoteProposer; -import tech.pegasys.pantheon.consensus.common.VoteTally; -import tech.pegasys.pantheon.consensus.common.VoteTallyUpdater; -import tech.pegasys.pantheon.ethereum.ProtocolContext; -import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain; -import tech.pegasys.pantheon.ethereum.core.Block; -import tech.pegasys.pantheon.ethereum.core.BlockBody; -import tech.pegasys.pantheon.ethereum.core.BlockHeaderTestFixture; -import tech.pegasys.pantheon.ethereum.core.BlockImporter; -import tech.pegasys.pantheon.ethereum.mainnet.HeaderValidationMode; -import tech.pegasys.pantheon.ethereum.worldstate.WorldStateArchive; - -import org.junit.Test; - -public class IbftBlockImporterTest { - - private final VoteTallyUpdater voteTallyUpdater = mock(VoteTallyUpdater.class); - private final VoteTally voteTally = mock(VoteTally.class); - private final VoteProposer voteProposer = mock(VoteProposer.class); - - @SuppressWarnings("unchecked") - private final BlockImporter delegate = mock(BlockImporter.class); - - private final MutableBlockchain blockchain = mock(MutableBlockchain.class); - private final WorldStateArchive worldStateArchive = mock(WorldStateArchive.class); - private final ProtocolContext context = - new ProtocolContext<>( - blockchain, worldStateArchive, new IbftContext(voteTally, voteProposer)); - - private final IbftBlockImporter importer = new IbftBlockImporter(delegate, voteTallyUpdater); - - @Test - public void voteTallyNotUpdatedWhenBlockImportFails() { - final BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture(); - final Block block = - new Block(headerBuilder.buildHeader(), new BlockBody(emptyList(), emptyList())); - - when(delegate.importBlock(context, block, HeaderValidationMode.FULL, HeaderValidationMode.FULL)) - .thenReturn(false); - - importer.importBlock(context, block, HeaderValidationMode.FULL); - - verifyZeroInteractions(voteTallyUpdater); - } - - @Test - public void voteTallyNotUpdatedWhenFastBlockImportFails() { - final BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture(); - final Block block = - new Block(headerBuilder.buildHeader(), new BlockBody(emptyList(), emptyList())); - - when(delegate.fastImportBlock(context, block, emptyList(), HeaderValidationMode.LIGHT)) - .thenReturn(false); - - importer.fastImportBlock(context, block, emptyList(), HeaderValidationMode.LIGHT); - - verifyZeroInteractions(voteTallyUpdater); - } - - @Test - public void voteTallyUpdatedWhenBlockImportSucceeds() { - final Block block = - new Block( - new BlockHeaderTestFixture().buildHeader(), new BlockBody(emptyList(), emptyList())); - - when(delegate.importBlock(context, block, HeaderValidationMode.FULL, HeaderValidationMode.FULL)) - .thenReturn(true); - - importer.importBlock(context, block, HeaderValidationMode.FULL); - - verify(voteTallyUpdater).updateForBlock(block.getHeader(), voteTally); - } - - @Test - public void voteTallyUpdatedWhenFastBlockImportSucceeds() { - final Block block = - new Block( - new BlockHeaderTestFixture().buildHeader(), new BlockBody(emptyList(), emptyList())); - - when(delegate.fastImportBlock(context, block, emptyList(), HeaderValidationMode.LIGHT)) - .thenReturn(true); - - importer.fastImportBlock(context, block, emptyList(), HeaderValidationMode.LIGHT); - - verify(voteTallyUpdater).updateForBlock(block.getHeader(), voteTally); - } -} diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorTest.java index df3526eb03..fb5e396668 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorTest.java @@ -16,10 +16,10 @@ 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.ibft.IbftContextBuilder.setupContextWithValidators; import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.createInMemoryWorldStateArchive; import tech.pegasys.pantheon.config.GenesisConfigFile; -import tech.pegasys.pantheon.consensus.common.VoteProposer; import tech.pegasys.pantheon.consensus.common.VoteTally; import tech.pegasys.pantheon.consensus.ibft.IbftBlockHashing; import tech.pegasys.pantheon.consensus.ibft.IbftBlockHeaderValidationRulesetFactory; @@ -78,7 +78,7 @@ public class IbftBlockCreatorTest { new ProtocolContext<>( blockchain, createInMemoryWorldStateArchive(), - new IbftContext(voteTally, new VoteProposer())); + setupContextWithValidators(initialValidatorList)); final IbftBlockCreator blockCreator = new IbftBlockCreator( diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/ProposerSelectorTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/ProposerSelectorTest.java index ebfeea366b..3ed50210fa 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/ProposerSelectorTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/ProposerSelectorTest.java @@ -19,7 +19,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import tech.pegasys.pantheon.consensus.common.BlockInterface; -import tech.pegasys.pantheon.consensus.common.VoteTally; import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; import tech.pegasys.pantheon.ethereum.chain.Blockchain; import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain; @@ -28,6 +27,7 @@ import tech.pegasys.pantheon.ethereum.core.AddressHelpers; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.core.BlockHeaderTestFixture; +import java.util.Collection; import java.util.LinkedList; import java.util.Optional; @@ -39,9 +39,10 @@ public class ProposerSelectorTest { private final BlockInterface blockInterface = mock(BlockInterface.class); private Blockchain createMockedBlockChainWithHeadOf( - final long blockNumber, final Address proposer) { + final long blockNumber, final Address proposer, final Collection
validators) { when(blockInterface.getProposerOfBlock(any())).thenReturn(proposer); + when(blockInterface.validatorsInBlock(any())).thenReturn(validators); final BlockHeaderTestFixture headerBuilderFixture = new BlockHeaderTestFixture(); headerBuilderFixture.number(blockNumber); @@ -87,12 +88,11 @@ public class ProposerSelectorTest { final long PREV_BLOCK_NUMBER = 2; final Address localAddr = AddressHelpers.ofValue(10); // arbitrarily selected - final Blockchain blockchain = createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr); - final LinkedList
validatorList = createValidatorList(localAddr, 0, 4); - final VoteTally voteTally = new VoteTally(validatorList); + final Blockchain blockchain = + createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr, validatorList); - final ProposerSelector uut = new ProposerSelector(blockchain, voteTally, blockInterface, true); + final ProposerSelector uut = new ProposerSelector(blockchain, blockInterface, true); final ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(PREV_BLOCK_NUMBER + 1, 0); @@ -105,12 +105,12 @@ public class ProposerSelectorTest { public void lastValidatorInListValidatedPreviousBlockSoFirstIsNextProposer() { final long PREV_BLOCK_NUMBER = 2; final Address localAddr = AddressHelpers.ofValue(10); // arbitrarily selected - final Blockchain blockchain = createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr); final LinkedList
validatorList = createValidatorList(localAddr, 4, 0); - final VoteTally voteTally = new VoteTally(validatorList); + final Blockchain blockchain = + createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr, validatorList); - final ProposerSelector uut = new ProposerSelector(blockchain, voteTally, blockInterface, true); + final ProposerSelector uut = new ProposerSelector(blockchain, blockInterface, true); final ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(PREV_BLOCK_NUMBER + 1, 0); @@ -125,11 +125,11 @@ public class ProposerSelectorTest { final ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(PREV_BLOCK_NUMBER + 1, 0); final Address localAddr = AddressHelpers.ofValue(10); // arbitrarily selected - final Blockchain blockchain = createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr); final LinkedList
validatorList = createValidatorList(localAddr, 4, 0); - final VoteTally voteTally = new VoteTally(validatorList); + final Blockchain blockchain = + createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr, validatorList); - final ProposerSelector uut = new ProposerSelector(blockchain, voteTally, blockInterface, false); + final ProposerSelector uut = new ProposerSelector(blockchain, blockInterface, false); final Address nextProposer = uut.selectProposerForRound(roundId); assertThat(nextProposer).isEqualTo(localAddr); @@ -141,12 +141,12 @@ public class ProposerSelectorTest { ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(PREV_BLOCK_NUMBER + 1, 0); final Address localAddr = AddressHelpers.ofValue(10); // arbitrarily selected - final Blockchain blockchain = createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr); final LinkedList
validatorList = createValidatorList(localAddr, 4, 0); - final VoteTally voteTally = new VoteTally(validatorList); + final Blockchain blockchain = + createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr, validatorList); - final ProposerSelector uut = new ProposerSelector(blockchain, voteTally, blockInterface, false); + final ProposerSelector uut = new ProposerSelector(blockchain, blockInterface, false); assertThat(uut.selectProposerForRound(roundId)).isEqualTo(localAddr); roundId = new ConsensusRoundIdentifier(PREV_BLOCK_NUMBER + 1, 1); @@ -162,7 +162,6 @@ public class ProposerSelectorTest { final ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(PREV_BLOCK_NUMBER + 1, 0); final Address localAddr = AddressHelpers.ofValue(10); // arbitrarily selected - final Blockchain blockchain = createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr); // LocalAddr will be in index 2 - the next proposer will also be in 2 (as prev proposer is // removed) @@ -170,9 +169,10 @@ public class ProposerSelectorTest { validatorList.remove(localAddr); // Note the signer of the Previous block was not included. - final VoteTally voteTally = new VoteTally(validatorList); + final Blockchain blockchain = + createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr, validatorList); - final ProposerSelector uut = new ProposerSelector(blockchain, voteTally, blockInterface, false); + final ProposerSelector uut = new ProposerSelector(blockchain, blockInterface, false); assertThat(uut.selectProposerForRound(roundId)).isEqualTo(validatorList.get(2)); } @@ -183,17 +183,16 @@ public class ProposerSelectorTest { final ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(PREV_BLOCK_NUMBER + 1, 0); final Address localAddr = AddressHelpers.ofValue(10); // arbitrarily selected - final Blockchain blockchain = createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr); // LocalAddr will be in index 2 - the next proposer will also be in 2 (as prev proposer is // removed) final LinkedList
validatorList = createValidatorList(localAddr, 2, 2); validatorList.remove(localAddr); - // Note the signer of the Previous block was not included. - final VoteTally voteTally = new VoteTally(validatorList); + final Blockchain blockchain = + createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr, validatorList); - final ProposerSelector uut = new ProposerSelector(blockchain, voteTally, blockInterface, true); + final ProposerSelector uut = new ProposerSelector(blockchain, blockInterface, true); assertThat(uut.selectProposerForRound(roundId)).isEqualTo(validatorList.get(2)); } @@ -204,7 +203,6 @@ public class ProposerSelectorTest { final ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(PREV_BLOCK_NUMBER + 1, 0); final Address localAddr = AddressHelpers.ofValue(10); // arbitrarily selected - final Blockchain blockchain = createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr); // LocalAddr will be in index 2 - the next proposer will also be in 2 (as prev proposer is // removed) @@ -212,9 +210,10 @@ public class ProposerSelectorTest { validatorList.remove(localAddr); // Note the signer of the Previous block was not included. - final VoteTally voteTally = new VoteTally(validatorList); + final Blockchain blockchain = + createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr, validatorList); - final ProposerSelector uut = new ProposerSelector(blockchain, voteTally, blockInterface, false); + final ProposerSelector uut = new ProposerSelector(blockchain, blockInterface, false); assertThat(uut.selectProposerForRound(roundId)).isEqualTo(validatorList.get(0)); } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCoinbaseValidationRuleTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCoinbaseValidationRuleTest.java index 7abeb7cc84..2d70e38502 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCoinbaseValidationRuleTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCoinbaseValidationRuleTest.java @@ -13,8 +13,8 @@ package tech.pegasys.pantheon.consensus.ibft.headervalidationrules; import static org.assertj.core.api.Assertions.assertThat; +import static tech.pegasys.pantheon.consensus.ibft.IbftContextBuilder.setupContextWithValidators; -import tech.pegasys.pantheon.consensus.common.VoteTally; import tech.pegasys.pantheon.consensus.ibft.IbftContext; import tech.pegasys.pantheon.consensus.ibft.IbftExtraData; import tech.pegasys.pantheon.consensus.ibft.IbftExtraDataFixture; @@ -68,9 +68,8 @@ public class IbftCoinbaseValidationRuleTest { final List committers = Lists.newArrayList(proposerKeyPair); - final VoteTally voteTally = new VoteTally(validators); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); final IbftCoinbaseValidationRule coinbaseValidationRule = new IbftCoinbaseValidationRule(); @@ -91,9 +90,8 @@ public class IbftCoinbaseValidationRuleTest { final List committers = Lists.newArrayList(otherValidatorKeyPair); - final VoteTally voteTally = new VoteTally(validators); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); final IbftCoinbaseValidationRule coinbaseValidationRule = new IbftCoinbaseValidationRule(); diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCommitSealsValidationRuleTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCommitSealsValidationRuleTest.java index f92cb6000b..96b881a971 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCommitSealsValidationRuleTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftCommitSealsValidationRuleTest.java @@ -15,9 +15,9 @@ package tech.pegasys.pantheon.consensus.ibft.headervalidationrules; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import static tech.pegasys.pantheon.consensus.ibft.IbftContextBuilder.setupContextWithValidators; import static tech.pegasys.pantheon.consensus.ibft.headervalidationrules.HeaderValidationTestHelpers.createProposedBlockHeader; -import tech.pegasys.pantheon.consensus.common.VoteTally; import tech.pegasys.pantheon.consensus.ibft.IbftContext; import tech.pegasys.pantheon.consensus.ibft.IbftExtraData; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; @@ -51,9 +51,8 @@ public class IbftCommitSealsValidationRuleTest { .sorted() .collect(Collectors.toList()); - final VoteTally voteTally = new VoteTally(committerAddresses); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(committerAddresses)); BlockHeader header = createProposedBlockHeader(committerAddresses, committerKeyPairs, false); @@ -67,9 +66,8 @@ public class IbftCommitSealsValidationRuleTest { Address.extract(Hash.hash(committerKeyPair.getPublicKey().getEncodedBytes())); final List
validators = singletonList(committerAddress); - final VoteTally voteTally = new VoteTally(validators); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); final BlockHeader header = createProposedBlockHeader(validators, emptyList(), false); @@ -86,16 +84,15 @@ public class IbftCommitSealsValidationRuleTest { final Address committerAddress = Util.publicKeyToAddress(committerKeyPair.getPublicKey()); final List
validators = singletonList(committerAddress); - final VoteTally voteTally = new VoteTally(validators); // Insert an extraData block with committer seals. final KeyPair nonValidatorKeyPair = KeyPair.generate(); - BlockHeader header = + final BlockHeader header = createProposedBlockHeader(validators, singletonList(nonValidatorKeyPair), false); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); assertThat(commitSealsValidationRule.validate(header, null, context)).isFalse(); } @@ -148,9 +145,8 @@ public class IbftCommitSealsValidationRuleTest { createProposedBlockHeader( validators, Lists.newArrayList(committerKeyPair, committerKeyPair), false); - final VoteTally voteTally = new VoteTally(validators); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); assertThat(commitSealsValidationRule.validate(header, null, context)).isFalse(); } @@ -170,7 +166,6 @@ public class IbftCommitSealsValidationRuleTest { } Collections.sort(validators); - final VoteTally voteTally = new VoteTally(validators); final BlockHeader header = createProposedBlockHeader( validators, @@ -178,7 +173,7 @@ public class IbftCommitSealsValidationRuleTest { useDifferentRoundNumbersForCommittedSeals); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); return commitSealsValidationRule.validate(header, null, context); } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftValidatorsValidationRuleTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftValidatorsValidationRuleTest.java index 5638b2e052..51147542ab 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftValidatorsValidationRuleTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/headervalidationrules/IbftValidatorsValidationRuleTest.java @@ -14,9 +14,9 @@ package tech.pegasys.pantheon.consensus.ibft.headervalidationrules; import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThat; +import static tech.pegasys.pantheon.consensus.ibft.IbftContextBuilder.setupContextWithValidators; import static tech.pegasys.pantheon.consensus.ibft.headervalidationrules.HeaderValidationTestHelpers.createProposedBlockHeader; -import tech.pegasys.pantheon.consensus.common.VoteTally; import tech.pegasys.pantheon.consensus.ibft.IbftContext; import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.core.Address; @@ -39,9 +39,8 @@ public class IbftValidatorsValidationRuleTest { Lists.newArrayList( AddressHelpers.ofValue(1), AddressHelpers.ofValue(2), AddressHelpers.ofValue(3)); - final VoteTally voteTally = new VoteTally(validators); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); final BlockHeader header = createProposedBlockHeader(validators, emptyList(), false); @@ -53,13 +52,13 @@ public class IbftValidatorsValidationRuleTest { final List
validators = Lists.newArrayList( - AddressHelpers.ofValue(3), AddressHelpers.ofValue(2), AddressHelpers.ofValue(1)); + AddressHelpers.ofValue(1), AddressHelpers.ofValue(2), AddressHelpers.ofValue(3)); - final VoteTally voteTally = new VoteTally(validators); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); - final BlockHeader header = createProposedBlockHeader(validators, emptyList(), false); + final BlockHeader header = + createProposedBlockHeader(Lists.reverse(validators), emptyList(), false); assertThat(validatorsValidationRule.validate(header, null, context)).isFalse(); } @@ -74,9 +73,8 @@ public class IbftValidatorsValidationRuleTest { Lists.newArrayList( AddressHelpers.ofValue(2), AddressHelpers.ofValue(3), AddressHelpers.ofValue(4)); - final VoteTally voteTally = new VoteTally(storedValidators); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(storedValidators)); final BlockHeader header = createProposedBlockHeader(reportedValidators, emptyList(), false); diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/ValidatorPeersTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/ValidatorPeersTest.java index 42d2e442fa..6b89a7a1d7 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/ValidatorPeersTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/ValidatorPeersTest.java @@ -20,7 +20,8 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import tech.pegasys.pantheon.consensus.common.ValidatorProvider; +import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.crypto.SECP256K1.PublicKey; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.Util; @@ -67,10 +68,12 @@ public class ValidatorPeersTest { public void onlyValidatorsAreSentAMessage() throws PeerNotConnected { // Only add the first Peer's address to the validators. validators.add(Util.publicKeyToAddress(publicKeys.get(0))); - final ValidatorProvider validatorProvider = mock(ValidatorProvider.class); + final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); + final VoteTally validatorProvider = mock(VoteTally.class); + when(voteTallyCache.getVoteTallyAtHead()).thenReturn(validatorProvider); when(validatorProvider.getValidators()).thenReturn(validators); - final ValidatorPeers peers = new ValidatorPeers(validatorProvider); + final ValidatorPeers peers = new ValidatorPeers(voteTallyCache); for (final PeerConnection peer : peerConnections) { peers.add(peer); } @@ -88,10 +91,12 @@ public class ValidatorPeersTest { public void doesntSendToValidatorsWhichAreNotDirectlyConnected() throws PeerNotConnected { validators.add(Util.publicKeyToAddress(publicKeys.get(0))); - final ValidatorProvider validatorProvider = mock(ValidatorProvider.class); + final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); + final VoteTally validatorProvider = mock(VoteTally.class); + when(voteTallyCache.getVoteTallyAtHead()).thenReturn(validatorProvider); when(validatorProvider.getValidators()).thenReturn(validators); - final ValidatorPeers peers = new ValidatorPeers(validatorProvider); + final ValidatorPeers peers = new ValidatorPeers(voteTallyCache); // only add peer connections 1, 2 & 3, none of which should be invoked. newArrayList(1, 2, 3).forEach(i -> peers.add(peerConnections.get(i))); @@ -111,10 +116,12 @@ public class ValidatorPeersTest { final Address validatorAddress = Util.publicKeyToAddress(publicKeys.get(0)); validators.add(validatorAddress); validators.add(Util.publicKeyToAddress(publicKeys.get(1))); - final ValidatorProvider validatorProvider = mock(ValidatorProvider.class); + final VoteTallyCache voteTallyCache = mock(VoteTallyCache.class); + final VoteTally validatorProvider = mock(VoteTally.class); + when(voteTallyCache.getVoteTallyAtHead()).thenReturn(validatorProvider); when(validatorProvider.getValidators()).thenReturn(validators); - final ValidatorPeers peers = new ValidatorPeers(validatorProvider); + final ValidatorPeers peers = new ValidatorPeers(voteTallyCache); for (final PeerConnection peer : peerConnections) { peers.add(peer); } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java index 4a8eb63bc3..a2b58c805a 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java @@ -24,9 +24,9 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static tech.pegasys.pantheon.consensus.ibft.IbftContextBuilder.setupContextWithValidators; import static tech.pegasys.pantheon.consensus.ibft.TestHelpers.createFrom; -import tech.pegasys.pantheon.consensus.common.VoteTally; import tech.pegasys.pantheon.consensus.ibft.BlockTimer; import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; import tech.pegasys.pantheon.consensus.ibft.IbftContext; @@ -135,12 +135,11 @@ public class IbftBlockHeightManagerTest { when(finalState.getMessageFactory()).thenReturn(messageFactory); when(blockCreator.createBlock(anyLong())).thenReturn(createdBlock); when(newRoundPayloadValidator.validateNewRoundMessage(any())).thenReturn(true); - when(messageValidatorFactory.createNewRoundValidator(anyLong())) + when(messageValidatorFactory.createNewRoundValidator(anyLong(), any())) .thenReturn(newRoundPayloadValidator); - when(messageValidatorFactory.createMessageValidator(any())).thenReturn(messageValidator); + when(messageValidatorFactory.createMessageValidator(any(), any())).thenReturn(messageValidator); - protocolContext = - new ProtocolContext<>(null, null, new IbftContext(new VoteTally(validators), null)); + protocolContext = new ProtocolContext<>(null, null, setupContextWithValidators(validators)); // Ensure the created IbftRound has the valid ConsensusRoundIdentifier; when(roundFactory.createNewRound(any(), anyInt())) diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundTest.java index c54f6ca2ae..b2068a0c50 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundTest.java @@ -22,9 +22,8 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static tech.pegasys.pantheon.consensus.ibft.IbftContextBuilder.setupContextWithValidators; -import tech.pegasys.pantheon.consensus.common.VoteProposer; -import tech.pegasys.pantheon.consensus.common.VoteTally; import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; import tech.pegasys.pantheon.consensus.ibft.IbftBlockHashing; import tech.pegasys.pantheon.consensus.ibft.IbftContext; @@ -94,9 +93,7 @@ public class IbftRoundTest { public void setup() { protocolContext = new ProtocolContext<>( - blockChain, - worldStateArchive, - new IbftContext(new VoteTally(emptyList()), new VoteProposer())); + blockChain, worldStateArchive, setupContextWithValidators(emptyList())); when(messageValidator.validateProposal(any())).thenReturn(true); when(messageValidator.validatePrepare(any())).thenReturn(true); diff --git a/consensus/ibftlegacy/src/main/java/tech/pegasys/pantheon/consensus/ibftlegacy/IbftProtocolSchedule.java b/consensus/ibftlegacy/src/main/java/tech/pegasys/pantheon/consensus/ibftlegacy/IbftProtocolSchedule.java index 1e45af85d0..7fa3199b8d 100644 --- a/consensus/ibftlegacy/src/main/java/tech/pegasys/pantheon/consensus/ibftlegacy/IbftProtocolSchedule.java +++ b/consensus/ibftlegacy/src/main/java/tech/pegasys/pantheon/consensus/ibftlegacy/IbftProtocolSchedule.java @@ -17,8 +17,6 @@ import static tech.pegasys.pantheon.consensus.ibftlegacy.IbftBlockHeaderValidati import tech.pegasys.pantheon.config.GenesisConfigOptions; import tech.pegasys.pantheon.config.IbftConfigOptions; import tech.pegasys.pantheon.consensus.common.EpochManager; -import tech.pegasys.pantheon.consensus.common.VoteTallyUpdater; -import tech.pegasys.pantheon.consensus.ibft.IbftBlockImporter; import tech.pegasys.pantheon.consensus.ibft.IbftContext; import tech.pegasys.pantheon.ethereum.MainnetBlockValidator; import tech.pegasys.pantheon.ethereum.core.PrivacyParameters; @@ -60,10 +58,7 @@ public class IbftProtocolSchedule { difficultyCalculator -> ibftBlockHeaderValidator(secondsBetweenBlocks), MainnetBlockBodyValidator::new, MainnetBlockValidator::new, - (blockValidator) -> - new IbftBlockImporter( - new MainnetBlockImporter<>(blockValidator), - new VoteTallyUpdater(epochManager, new IbftLegacyBlockInterface())), + MainnetBlockImporter::new, (time, parent, protocolContext) -> BigInteger.ONE) .blockReward(Wei.ZERO) .blockHashFunction(IbftBlockHashing::calculateHashOfIbftBlockOnChain); 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 b1aaf78e62..fecc3a1e04 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 @@ -25,6 +25,8 @@ import tech.pegasys.pantheon.ethereum.rlp.RLPException; import java.util.Collection; import java.util.List; +import java.util.SortedSet; +import java.util.TreeSet; import com.google.common.collect.Iterables; import org.apache.logging.log4j.LogManager; @@ -50,26 +52,9 @@ public class IbftExtraDataValidationRule implements AttachedBlockHeaderValidatio final BlockHeader header, final BlockHeader parent, final ProtocolContext context) { - return validateExtraData(header, context); - } - - /** - * Responsible for determining the validity of the extra data field. Ensures: - * - *
    - *
  • Bytes in the extra data field can be decoded as per IBFT specification - *
  • Proposer (derived from the proposerSeal) is a member of the validators - *
  • Committers (derived from committerSeals) are all members of the validators - *
- * - * @param header the block header containing the extraData to be validated. - * @return True if the extraData successfully produces an IstanbulExtraData object, false - * otherwise - */ - private boolean validateExtraData( - final BlockHeader header, final ProtocolContext context) { try { - final ValidatorProvider validatorProvider = context.getConsensusState().getVoteTally(); + final ValidatorProvider validatorProvider = + context.getConsensusState().getVoteTallyCache().getVoteTallyAfterBlock(parent); final IbftExtraData ibftExtraData = IbftExtraData.decode(header.getExtraData()); final Address proposer = IbftBlockHashing.recoverProposerAddress(header, ibftExtraData); @@ -89,6 +74,17 @@ public class IbftExtraDataValidationRule implements AttachedBlockHeaderValidatio } } + final SortedSet
sortedReportedValidators = + new TreeSet<>(ibftExtraData.getValidators()); + + if (!Iterables.elementsEqual(ibftExtraData.getValidators(), sortedReportedValidators)) { + LOG.trace( + "Validators are not sorted in ascending order. Expected {} but got {}.", + sortedReportedValidators, + ibftExtraData.getValidators()); + return false; + } + if (!Iterables.elementsEqual(ibftExtraData.getValidators(), storedValidators)) { LOG.trace( "Incorrect validators. Expected {} but got {}.", @@ -103,6 +99,9 @@ public class IbftExtraDataValidationRule implements AttachedBlockHeaderValidatio } catch (final IllegalArgumentException ex) { LOG.trace("Failed to verify extra data", ex); return false; + } catch (final RuntimeException ex) { + LOG.trace("Failed to find validators at parent"); + return false; } return true; diff --git a/consensus/ibftlegacy/src/test/java/tech/pegasys/pantheon/consensus/ibftlegacy/IbftBlockHeaderValidationRulesetFactoryTest.java b/consensus/ibftlegacy/src/test/java/tech/pegasys/pantheon/consensus/ibftlegacy/IbftBlockHeaderValidationRulesetFactoryTest.java index 4a842db60e..fd44e0b010 100644 --- a/consensus/ibftlegacy/src/test/java/tech/pegasys/pantheon/consensus/ibftlegacy/IbftBlockHeaderValidationRulesetFactoryTest.java +++ b/consensus/ibftlegacy/src/test/java/tech/pegasys/pantheon/consensus/ibftlegacy/IbftBlockHeaderValidationRulesetFactoryTest.java @@ -15,12 +15,17 @@ package tech.pegasys.pantheon.consensus.ibftlegacy; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; 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 tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.consensus.ibft.IbftContext; -import tech.pegasys.pantheon.consensus.ibft.IbftProtocolContextFixture; 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.BlockHeader; import tech.pegasys.pantheon.ethereum.core.BlockHeaderTestFixture; @@ -31,12 +36,25 @@ import tech.pegasys.pantheon.util.bytes.BytesValue; import tech.pegasys.pantheon.util.uint.UInt256; import java.math.BigInteger; +import java.util.Collection; import java.util.List; import org.junit.Test; public class IbftBlockHeaderValidationRulesetFactoryTest { + private ProtocolContext setupContextWithValidators( + final Collection
validators) { + final IbftContext ibftContext = mock(IbftContext.class); + final VoteTallyCache mockCache = mock(VoteTallyCache.class); + final VoteTally mockVoteTally = mock(VoteTally.class); + when(ibftContext.getVoteTallyCache()).thenReturn(mockCache); + when(mockCache.getVoteTallyAfterBlock(any())).thenReturn(mockVoteTally); + when(mockVoteTally.getValidators()).thenReturn(validators); + + return new ProtocolContext<>(null, null, ibftContext); + } + @Test public void ibftValidateHeaderPasses() { final KeyPair proposerKeyPair = KeyPair.generate(); @@ -55,7 +73,7 @@ public class IbftBlockHeaderValidationRulesetFactoryTest { validator.validateHeader( blockHeader, parentHeader, - IbftProtocolContextFixture.protocolContext(validators), + setupContextWithValidators(validators), HeaderValidationMode.FULL)) .isTrue(); } @@ -78,7 +96,7 @@ public class IbftBlockHeaderValidationRulesetFactoryTest { validator.validateHeader( blockHeader, parentHeader, - IbftProtocolContextFixture.protocolContext(validators), + setupContextWithValidators(validators), HeaderValidationMode.FULL)) .isFalse(); } diff --git a/consensus/ibftlegacy/src/test/java/tech/pegasys/pantheon/consensus/ibftlegacy/blockcreation/IbftBlockCreatorTest.java b/consensus/ibftlegacy/src/test/java/tech/pegasys/pantheon/consensus/ibftlegacy/blockcreation/IbftBlockCreatorTest.java index b9553a696d..c5ffe7bd9b 100644 --- a/consensus/ibftlegacy/src/test/java/tech/pegasys/pantheon/consensus/ibftlegacy/blockcreation/IbftBlockCreatorTest.java +++ b/consensus/ibftlegacy/src/test/java/tech/pegasys/pantheon/consensus/ibftlegacy/blockcreation/IbftBlockCreatorTest.java @@ -16,10 +16,10 @@ 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.ibft.IbftContextBuilder.setupContextWithValidators; import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.createInMemoryWorldStateArchive; import tech.pegasys.pantheon.config.GenesisConfigFile; -import tech.pegasys.pantheon.consensus.common.VoteProposer; import tech.pegasys.pantheon.consensus.common.VoteTally; import tech.pegasys.pantheon.consensus.ibft.IbftContext; import tech.pegasys.pantheon.consensus.ibftlegacy.IbftBlockHeaderValidationRulesetFactory; @@ -84,7 +84,7 @@ public class IbftBlockCreatorTest { new ProtocolContext<>( blockchain, createInMemoryWorldStateArchive(), - new IbftContext(voteTally, new VoteProposer())); + setupContextWithValidators(initialValidatorList)); final IbftBlockCreator blockCreator = new IbftBlockCreator( diff --git a/consensus/ibftlegacy/src/test/java/tech/pegasys/pantheon/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRuleTest.java b/consensus/ibftlegacy/src/test/java/tech/pegasys/pantheon/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRuleTest.java index 40c657ed8b..a8a620041b 100644 --- a/consensus/ibftlegacy/src/test/java/tech/pegasys/pantheon/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRuleTest.java +++ b/consensus/ibftlegacy/src/test/java/tech/pegasys/pantheon/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRuleTest.java @@ -15,6 +15,7 @@ package tech.pegasys.pantheon.consensus.ibftlegacy.headervalidationrules; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import static tech.pegasys.pantheon.consensus.ibft.IbftContextBuilder.setupContextWithValidators; import tech.pegasys.pantheon.consensus.common.VoteTally; import tech.pegasys.pantheon.consensus.ibft.IbftContext; @@ -103,9 +104,8 @@ public class IbftExtraDataValidationRuleTest { Address.extract(Hash.hash(proposerKeyPair.getPublicKey().getEncodedBytes())); final List
validators = singletonList(proposerAddress); - final VoteTally voteTally = new VoteTally(validators); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); final IbftExtraDataValidationRule extraDataValidationRule = new IbftExtraDataValidationRule(true); @@ -130,9 +130,8 @@ public class IbftExtraDataValidationRuleTest { Address.extract(Hash.hash(proposerKeyPair.getPublicKey().getEncodedBytes())); final List
validators = singletonList(proposerAddress); - final VoteTally voteTally = new VoteTally(validators); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); final IbftExtraDataValidationRule extraDataValidationRule = new IbftExtraDataValidationRule(true); @@ -158,9 +157,8 @@ public class IbftExtraDataValidationRuleTest { Lists.newArrayList( AddressHelpers.calculateAddressWithRespectTo(proposerAddress, 1), proposerAddress); - final VoteTally voteTally = new VoteTally(validators); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); final IbftExtraDataValidationRule extraDataValidationRule = new IbftExtraDataValidationRule(true); @@ -190,7 +188,7 @@ public class IbftExtraDataValidationRuleTest { final VoteTally voteTally = new VoteTally(validators); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); final IbftExtraDataValidationRule extraDataValidationRule = new IbftExtraDataValidationRule(true); @@ -216,16 +214,17 @@ public class IbftExtraDataValidationRuleTest { final List
validators = Lists.newArrayList(proposerAddress); - final VoteTally voteTally = new VoteTally(validators); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); final IbftExtraDataValidationRule extraDataValidationRule = new IbftExtraDataValidationRule(true); // Add another validator to the list reported in the IbftExtraData (note, as the - validators.add(AddressHelpers.calculateAddressWithRespectTo(proposerAddress, 1)); - BlockHeader header = createProposedBlockHeader(proposerKeyPair, validators); + final List
extraDataValidators = + Lists.newArrayList( + proposerAddress, AddressHelpers.calculateAddressWithRespectTo(proposerAddress, 1)); + BlockHeader header = createProposedBlockHeader(proposerKeyPair, extraDataValidators); // Insert an extraData block with committer seals. final IbftExtraData commitedExtraData = @@ -245,7 +244,6 @@ public class IbftExtraDataValidationRuleTest { Address.extract(Hash.hash(proposerKeyPair.getPublicKey().getEncodedBytes())); final List
validators = singletonList(proposerAddress); - final VoteTally voteTally = new VoteTally(validators); BlockHeader header = createProposedBlockHeader(proposerKeyPair, validators); @@ -257,7 +255,7 @@ public class IbftExtraDataValidationRuleTest { header = builder.buildHeader(); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); final IbftExtraDataValidationRule extraDataValidationRule = new IbftExtraDataValidationRule(true); @@ -323,7 +321,7 @@ public class IbftExtraDataValidationRuleTest { header = builder.buildHeader(); final ProtocolContext context = - new ProtocolContext<>(null, null, new IbftContext(voteTally, null)); + new ProtocolContext<>(null, null, setupContextWithValidators(validators)); final IbftExtraDataValidationRule extraDataValidationRule = new IbftExtraDataValidationRule(true); diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/controller/CliquePantheonController.java b/pantheon/src/main/java/tech/pegasys/pantheon/controller/CliquePantheonController.java index 111d1cf852..a608d0d592 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/controller/CliquePantheonController.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/controller/CliquePantheonController.java @@ -20,13 +20,13 @@ import tech.pegasys.pantheon.consensus.clique.CliqueBlockInterface; import tech.pegasys.pantheon.consensus.clique.CliqueContext; import tech.pegasys.pantheon.consensus.clique.CliqueMiningTracker; import tech.pegasys.pantheon.consensus.clique.CliqueProtocolSchedule; -import tech.pegasys.pantheon.consensus.clique.VoteTallyCache; import tech.pegasys.pantheon.consensus.clique.blockcreation.CliqueBlockScheduler; import tech.pegasys.pantheon.consensus.clique.blockcreation.CliqueMinerExecutor; import tech.pegasys.pantheon.consensus.clique.blockcreation.CliqueMiningCoordinator; import tech.pegasys.pantheon.consensus.clique.jsonrpc.CliqueJsonRpcMethodsFactory; import tech.pegasys.pantheon.consensus.common.EpochManager; import tech.pegasys.pantheon.consensus.common.VoteProposer; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.VoteTallyUpdater; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.ethereum.ProtocolContext; @@ -113,7 +113,7 @@ public class CliquePantheonController implements PantheonController protocolSchedule = CliqueProtocolSchedule.create(genesisConfig.getConfigOptions(), nodeKeys); final GenesisState genesisState = GenesisState.fromConfig(genesisConfig, protocolSchedule); @@ -128,10 +128,11 @@ public class CliquePantheonController implements PantheonController { final EpochManager epochManager = new EpochManager(ibftConfig.getEpochLength()); - final VoteTally voteTally = - new VoteTallyUpdater(epochManager, new IbftLegacyBlockInterface()) - .buildVoteTallyFromBlockchain(blockchain); + final VoteTallyCache voteTallyCache = + new VoteTallyCache( + blockchain, + new VoteTallyUpdater(epochManager, new IbftLegacyBlockInterface()), + epochManager, + new IbftLegacyBlockInterface()); final VoteProposer voteProposer = new VoteProposer(); - return new IbftContext(voteTally, voteProposer); + return new IbftContext(voteTallyCache, voteProposer); }); final MutableBlockchain blockchain = protocolContext.getBlockchain(); diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java b/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java index ae272b75c6..1c33574f0c 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java @@ -19,7 +19,7 @@ import tech.pegasys.pantheon.config.IbftConfigOptions; import tech.pegasys.pantheon.consensus.common.BlockInterface; import tech.pegasys.pantheon.consensus.common.EpochManager; import tech.pegasys.pantheon.consensus.common.VoteProposer; -import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.common.VoteTallyCache; import tech.pegasys.pantheon.consensus.common.VoteTallyUpdater; import tech.pegasys.pantheon.consensus.ibft.BlockTimer; import tech.pegasys.pantheon.consensus.ibft.EventMultiplexer; @@ -145,14 +145,15 @@ public class IbftPantheonController implements PantheonController { metricsSystem, (blockchain, worldStateArchive) -> { final EpochManager epochManager = new EpochManager(ibftConfig.getEpochLength()); - final VoteTally voteTally = - new VoteTallyUpdater(epochManager, blockInterface) - .buildVoteTallyFromBlockchain(blockchain); - final VoteProposer voteProposer = new VoteProposer(); - return new IbftContext(voteTally, voteProposer); + return new IbftContext( + new VoteTallyCache( + blockchain, + new VoteTallyUpdater(epochManager, new IbftBlockInterface()), + epochManager, + new IbftBlockInterface()), + new VoteProposer()); }); final MutableBlockchain blockchain = protocolContext.getBlockchain(); - final VoteTally voteTally = protocolContext.getConsensusState().getVoteTally(); final boolean fastSyncEnabled = syncConfig.syncMode().equals(SyncMode.FAST); final EthProtocolManager ethProtocolManager = @@ -197,11 +198,12 @@ public class IbftPantheonController implements PantheonController { Util.publicKeyToAddress(nodeKeys.getPublicKey())); final ProposerSelector proposerSelector = - new ProposerSelector(blockchain, voteTally, blockInterface, true); + new ProposerSelector(blockchain, blockInterface, true); // NOTE: peers should not be used for accessing the network as it does not enforce the // "only send once" filter applied by the UniqueMessageMulticaster. - final ValidatorPeers peers = new ValidatorPeers(voteTally); + final VoteTallyCache voteTallyCache = protocolContext.getConsensusState().getVoteTallyCache(); + final ValidatorPeers peers = new ValidatorPeers(voteTallyCache); final UniqueMessageMulticaster uniqueMessageMulticaster = new UniqueMessageMulticaster(peers, ibftConfig.getGossipedHistoryLimit()); @@ -210,7 +212,7 @@ public class IbftPantheonController implements PantheonController { final IbftFinalState finalState = new IbftFinalState( - voteTally, + voteTallyCache, nodeKeys, Util.publicKeyToAddress(nodeKeys.getPublicKey()), proposerSelector, diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/util/BlockImporter.java b/pantheon/src/main/java/tech/pegasys/pantheon/util/BlockImporter.java index 0cc6c38455..e4e539e9b5 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/util/BlockImporter.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/util/BlockImporter.java @@ -162,7 +162,7 @@ public class BlockImporter { final BlockHeaderValidator blockHeaderValidator = protocolSpec.getBlockHeaderValidator(); final boolean validHeader = blockHeaderValidator.validateHeader( - header, previousHeader, context, HeaderValidationMode.FULL); + header, previousHeader, context, HeaderValidationMode.DETACHED_ONLY); if (!validHeader) { throw new IllegalStateException("Invalid header at block number " + header.getNumber() + "."); } @@ -177,7 +177,7 @@ public class BlockImporter { final tech.pegasys.pantheon.ethereum.core.BlockImporter blockImporter = protocolSpec.getBlockImporter(); final boolean blockImported = - blockImporter.importBlock(context, block, HeaderValidationMode.NONE); + blockImporter.importBlock(context, block, HeaderValidationMode.SKIP_DETACHED); if (!blockImported) { throw new IllegalStateException( "Invalid block at block number " + header.getNumber() + ".");