From 4bc6f6edf47a856e9aa150bdd6bd0965c8135365 Mon Sep 17 00:00:00 2001 From: tmohay Date: Tue, 19 Feb 2019 16:07:23 +1100 Subject: [PATCH 1/6] Failing import --- .../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 | 20 +--- .../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 | 39 +++++++ .../ibft/IbftProtocolContextFixture.java | 28 ----- ...ockHeaderValidationRulesetFactoryTest.java | 8 +- .../consensus/ibft/IbftBlockImporterTest.java | 107 ------------------ .../blockcreation/IbftBlockCreatorTest.java | 4 +- .../blockcreation/ProposerSelectorTest.java | 14 +-- .../IbftCoinbaseValidationRuleTest.java | 8 +- .../IbftCommitSealsValidationRuleTest.java | 19 ++-- .../IbftValidatorsValidationRuleTest.java | 11 +- .../ibft/network/ValidatorPeersTest.java | 21 ++-- .../IbftBlockHeightManagerTest.java | 9 +- .../ibft/statemachine/IbftRoundTest.java | 7 +- .../ibftlegacy/IbftProtocolSchedule.java | 7 +- .../IbftExtraDataValidationRule.java | 21 +--- ...ockHeaderValidationRulesetFactoryTest.java | 23 +++- .../blockcreation/IbftBlockCreatorTest.java | 4 +- .../IbftExtraDataValidationRuleTest.java | 18 ++- .../controller/CliquePantheonController.java | 13 ++- .../IbftLegacyPantheonController.java | 13 ++- .../controller/IbftPantheonController.java | 22 ++-- .../pegasys/pantheon/util/BlockImporter.java | 2 +- 57 files changed, 306 insertions(+), 478 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 @@ 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 @@ 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 @@ 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 @@ 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.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 @@ private VoteTallyCache createVoteTallyCache( 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 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 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.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.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.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 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.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 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 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.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.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 @@ private VoteTally populateCacheUptoAndIncluding(final BlockHeader start) { 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 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.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 @@ 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 @@ private Block createEmptyBlock(final long blockNumber, final Hash parentHash) { 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 void constructThreeBlockChain() { 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 void parentBlockVoteTallysAreCachedWhenChildVoteTallyRequested() { 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 void exceptionThrownIfNoParentBlockExists() { 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 void walkBackStopsWhenACachedVoteTallyIsFound() { @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 void integrationTestingVotesBeingApplied() { 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.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 @@ private static ControllerAndState createControllerAndFinalState( 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 @@ private static ControllerAndState createControllerAndFinalState( 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 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 static ProtocolSchedule create(final GenesisConfigOptions co 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 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 Wei getMinTransactionGasPrice() { 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.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 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 Address selectProposerForRound(final ConsensusRoundIdentifier roundIdenti *

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 @@ private Address handleMissingProposer( // 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 boolean validate( 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 boolean validate( 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..5d4436f08e 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 @@ -41,25 +41,9 @@ public boolean validate( 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(); 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 void remove(final PeerConnection removedConnection) { @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 @@ private void sendMessageToSpecificAddresses( } }); } + + 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 IbftBlockHeightManager( 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 @@ private BlockHeightManager createFullBlockHeightManager(final BlockHeader parent 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 @@ 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 @@ /** 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 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 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 IbftRound createNewRound(final BlockHeader parentHeader, final int round) 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.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 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 RoundChangeMessageValidator createRoundChangeMessageValidator(final long 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..66927fb79c --- /dev/null +++ b/consensus/ibft/src/test-support/java/tech/pegasys/pantheon/consensus/ibft/IbftContextBuilder.java @@ -0,0 +1,39 @@ +/* + * 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 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(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); + 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..d9a4c07d28 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 @@ 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.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 @@ public class IbftBlockHeaderValidationRulesetFactoryTest { + private final ProtocolContext protocolContext(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.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 void createdBlockPassesValidationRulesAndHasAppropriateHashAndMixHash() { 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..c5c1cb6b3d 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 @@ -92,7 +92,7 @@ public void roundRobinChangesProposerOnRoundZeroOfNextBlock() { final LinkedList
validatorList = createValidatorList(localAddr, 0, 4); final VoteTally voteTally = new VoteTally(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); @@ -110,7 +110,7 @@ public void lastValidatorInListValidatedPreviousBlockSoFirstIsNextProposer() { final LinkedList
validatorList = createValidatorList(localAddr, 4, 0); final VoteTally voteTally = new VoteTally(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); @@ -129,7 +129,7 @@ public void stickyProposerDoesNotChangeOnRoundZeroOfNextBlock() { final LinkedList
validatorList = createValidatorList(localAddr, 4, 0); final VoteTally voteTally = new VoteTally(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); @@ -146,7 +146,7 @@ public void stickyProposerChangesOnSubsequentRoundsAtSameBlockHeight() { final LinkedList
validatorList = createValidatorList(localAddr, 4, 0); final VoteTally voteTally = new VoteTally(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); @@ -172,7 +172,7 @@ public void whenProposerSelfRemovesSelectsNextProposerInLineEvenWhenSticky() { // Note the signer of the Previous block was not included. final VoteTally voteTally = new VoteTally(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)); } @@ -193,7 +193,7 @@ public void whenProposerSelfRemovesSelectsNextProposerInLineEvenWhenRoundRobin() // Note the signer of the Previous block was not included. final VoteTally voteTally = new VoteTally(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)); } @@ -214,7 +214,7 @@ public void proposerSelfRemovesAndHasHighestAddressNewProposerIsFirstInList() { // Note the signer of the Previous block was not included. final VoteTally voteTally = new VoteTally(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 void proposerInValidatorListPassesValidation() { 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 void proposerNotInValidatorListFailsValidation() { 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 @@ 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 void correctlyConstructedHeaderPassesValidation() { .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 void insufficientCommitSealsFailsValidation() { 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 void committerNotInValidatorListFailsValidation() { 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 void headerContainsDuplicateSealsFailsValidation() { 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 @@ private boolean subExecution( } Collections.sort(validators); - final VoteTally voteTally = new VoteTally(validators); final BlockHeader header = createProposedBlockHeader( validators, @@ -178,7 +173,7 @@ private boolean subExecution( 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..95e313e398 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 @@ 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 void correctlyConstructedHeaderPassesValidation() { 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); @@ -55,9 +54,8 @@ public void validatorsInNonAscendingOrderFailValidation() { Lists.newArrayList( AddressHelpers.ofValue(3), AddressHelpers.ofValue(2), AddressHelpers.ofValue(1)); - 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); @@ -74,9 +72,8 @@ public void mismatchingReportedValidatorsVsLocallyStoredListFailsValidation() { 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.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 void setup() { 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 void onlyValidatorsAreSentAMessage() throws PeerNotConnected { 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 void onlyValidatorsAreSentAMessageNotInExcludes() throws PeerNotConnected 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.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 void setup() { 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.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 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 @@ private static ProtocolSpecBuilder applyIbftChanges( 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..8c436190b2 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 @@ -50,26 +50,9 @@ public boolean validate( 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); 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..afd68a218d 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 @@ 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,24 @@ 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(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 +72,7 @@ public void ibftValidateHeaderPasses() { validator.validateHeader( blockHeader, parentHeader, - IbftProtocolContextFixture.protocolContext(validators), + setupContextWithValidators(validators), HeaderValidationMode.FULL)) .isTrue(); } @@ -78,7 +95,7 @@ public void ibftValidateHeaderFails() { 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.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 void headerProducedPassesValidationRules() { 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..121d405a85 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 @@ 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 void correctlyConstructedHeaderPassesValidation() { 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 void insufficientCommitSealsFailsValidation() { 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); @@ -160,7 +159,7 @@ public void outOfOrderValidatorListFailsValidation() { 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 +189,7 @@ public void proposerNotInValidatorListFailsValidation() { 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); @@ -218,7 +217,7 @@ public void mismatchingReportedValidatorsVsLocallyStoredListFailsValidation() { 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); @@ -245,7 +244,6 @@ public void committerNotInValidatorListFailsValidation() { 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 void committerNotInValidatorListFailsValidation() { 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 @@ private boolean subExecution(final int validatorCount, final int committerCount) 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.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 static PantheonController init( final long blocksPerEpoch = cliqueConfig.getEpochLength(); final long secondsBetweenBlocks = cliqueConfig.getBlockPeriodSeconds(); - final EpochManager epochManger = new EpochManager(blocksPerEpoch); + final EpochManager epochManager = new EpochManager(blocksPerEpoch); final ProtocolSchedule protocolSchedule = CliqueProtocolSchedule.create(genesisConfig.getConfigOptions(), nodeKeys); final GenesisState genesisState = GenesisState.fromConfig(genesisConfig, protocolSchedule); @@ -128,10 +128,11 @@ public static PantheonController init( new CliqueContext( new VoteTallyCache( blockchain, - new VoteTallyUpdater(epochManger, new CliqueBlockInterface()), - epochManger), + new VoteTallyUpdater(epochManager, new CliqueBlockInterface()), + epochManager, + new CliqueBlockInterface()), new VoteProposer(), - epochManger)); + epochManager)); final MutableBlockchain blockchain = protocolContext.getBlockchain(); final boolean fastSyncEnabled = syncConfig.syncMode().equals(SyncMode.FAST); @@ -176,7 +177,7 @@ public static PantheonController init( protocolContext.getConsensusState().getVoteTallyCache(), localAddress, secondsBetweenBlocks), - epochManger); + epochManager); final CliqueMiningCoordinator miningCoordinator = new CliqueMiningCoordinator( blockchain, diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftLegacyPantheonController.java b/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftLegacyPantheonController.java index 7c92cd82b0..d40fbf5fd0 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftLegacyPantheonController.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftLegacyPantheonController.java @@ -18,7 +18,7 @@ import tech.pegasys.pantheon.config.IbftConfigOptions; 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.IbftContext; import tech.pegasys.pantheon.consensus.ibft.jsonrpc.IbftJsonRpcMethodsFactory; @@ -112,12 +112,15 @@ public static PantheonController init( metricsSystem, (blockchain, worldStateArchive) -> { 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 9aac539eb5..9c74e245ee 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.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; @@ -143,14 +143,15 @@ public static PantheonController init( 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 = @@ -195,11 +196,12 @@ public static PantheonController init( 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()); @@ -208,7 +210,7 @@ public static PantheonController init( 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..b995bfe6e5 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/util/BlockImporter.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/util/BlockImporter.java @@ -81,7 +81,7 @@ public BlockImporter.ImportResult importBlockchain( if (header.getNumber() == BlockHeader.GENESIS_BLOCK_NUMBER) { continue; } - if (header.getNumber() % 100 == 0) { + if (header.getNumber() % 1 == 0) { LOG.info("Import at block {}", header.getNumber()); } if (blockchain.contains(header.getHash())) { From fbc87ba202260d77a51213526b8a27e088009515 Mon Sep 17 00:00:00 2001 From: tmohay Date: Tue, 19 Feb 2019 17:35:43 +1100 Subject: [PATCH 2/6] IBFT to use VoteTallyCache 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. --- .../IbftValidatorsValidationRule.java | 14 +++++++- .../consensus/ibft/IbftContextBuilder.java | 9 ++--- ...ockHeaderValidationRulesetFactoryTest.java | 2 +- .../blockcreation/ProposerSelectorTest.java | 35 +++++++++---------- .../IbftValidatorsValidationRuleTest.java | 5 +-- .../IbftExtraDataValidationRule.java | 16 +++++++++ ...ockHeaderValidationRulesetFactoryTest.java | 3 +- .../IbftExtraDataValidationRuleTest.java | 8 ++--- .../pegasys/pantheon/util/BlockImporter.java | 4 +-- 9 files changed, 63 insertions(+), 33 deletions(-) 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 5d4436f08e..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.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; @@ -46,8 +48,18 @@ public boolean validate( 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/test-support/java/tech/pegasys/pantheon/consensus/ibft/IbftContextBuilder.java b/consensus/ibft/src/test-support/java/tech/pegasys/pantheon/consensus/ibft/IbftContextBuilder.java index 66927fb79c..e9c0aea359 100644 --- 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 @@ -15,6 +15,7 @@ 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; @@ -25,10 +26,10 @@ public class IbftContextBuilder { - public static IbftContext setupContextWithValidators(Collection
validators) { - final IbftContext ibftContext = mock(IbftContext.class); - final VoteTallyCache mockCache = mock(VoteTallyCache.class); - final VoteTally mockVoteTally = mock(VoteTally.class); + 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); 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 d9a4c07d28..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 @@ -37,7 +37,7 @@ public class IbftBlockHeaderValidationRulesetFactoryTest { - private final ProtocolContext protocolContext(Collection
validators) { + private final ProtocolContext protocolContext(final Collection
validators) { return new ProtocolContext<>(null, null, setupContextWithValidators(validators)); } 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 c5c1cb6b3d..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.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.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,10 +88,9 @@ public void roundRobinChangesProposerOnRoundZeroOfNextBlock() { 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, blockInterface, true); @@ -105,10 +105,10 @@ public void roundRobinChangesProposerOnRoundZeroOfNextBlock() { 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, blockInterface, true); @@ -125,9 +125,9 @@ public void stickyProposerDoesNotChangeOnRoundZeroOfNextBlock() { 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, blockInterface, false); final Address nextProposer = uut.selectProposerForRound(roundId); @@ -141,10 +141,10 @@ public void stickyProposerChangesOnSubsequentRoundsAtSameBlockHeight() { 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, blockInterface, false); assertThat(uut.selectProposerForRound(roundId)).isEqualTo(localAddr); @@ -162,7 +162,6 @@ public void whenProposerSelfRemovesSelectsNextProposerInLineEvenWhenSticky() { 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,7 +169,8 @@ public void whenProposerSelfRemovesSelectsNextProposerInLineEvenWhenSticky() { 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, blockInterface, false); @@ -183,15 +183,14 @@ public void whenProposerSelfRemovesSelectsNextProposerInLineEvenWhenRoundRobin() 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, blockInterface, true); @@ -204,7 +203,6 @@ public void proposerSelfRemovesAndHasHighestAddressNewProposerIsFirstInList() { 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,7 +210,8 @@ public void proposerSelfRemovesAndHasHighestAddressNewProposerIsFirstInList() { 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, blockInterface, false); 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 95e313e398..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 @@ -52,12 +52,13 @@ public void validatorsInNonAscendingOrderFailValidation() { final List
validators = Lists.newArrayList( - AddressHelpers.ofValue(3), AddressHelpers.ofValue(2), AddressHelpers.ofValue(1)); + AddressHelpers.ofValue(1), AddressHelpers.ofValue(2), AddressHelpers.ofValue(3)); final ProtocolContext context = 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(); } 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 8c436190b2..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 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; @@ -72,6 +74,17 @@ public boolean validate( } } + 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 {}.", @@ -86,6 +99,9 @@ public boolean validate( } 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 afd68a218d..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 @@ -43,7 +43,8 @@ public class IbftBlockHeaderValidationRulesetFactoryTest { - private ProtocolContext setupContextWithValidators(Collection
validators) { + private ProtocolContext setupContextWithValidators( + final Collection
validators) { final IbftContext ibftContext = mock(IbftContext.class); final VoteTallyCache mockCache = mock(VoteTallyCache.class); final VoteTally mockVoteTally = mock(VoteTally.class); 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 121d405a85..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 @@ -157,7 +157,6 @@ public void outOfOrderValidatorListFailsValidation() { Lists.newArrayList( AddressHelpers.calculateAddressWithRespectTo(proposerAddress, 1), proposerAddress); - final VoteTally voteTally = new VoteTally(validators); final ProtocolContext context = new ProtocolContext<>(null, null, setupContextWithValidators(validators)); @@ -215,7 +214,6 @@ public void mismatchingReportedValidatorsVsLocallyStoredListFailsValidation() { final List
validators = Lists.newArrayList(proposerAddress); - final VoteTally voteTally = new VoteTally(validators); final ProtocolContext context = new ProtocolContext<>(null, null, setupContextWithValidators(validators)); @@ -223,8 +221,10 @@ public void mismatchingReportedValidatorsVsLocallyStoredListFailsValidation() { 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 = 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 b995bfe6e5..cd6a6d282e 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 @@ private void validateBlock( 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 @@ private void evaluateBlock( 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() + "."); From e45ff7951daf133de6924fcaa2a31e3470668948 Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Wed, 20 Feb 2019 12:24:39 +1000 Subject: [PATCH 3/6] Reduce "Received transactions message" log from debug to trace since it's completely routine. (#919) --- .../ethereum/eth/transactions/TransactionsMessageProcessor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionsMessageProcessor.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionsMessageProcessor.java index 0cd4f6ce3a..1200ad29cc 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionsMessageProcessor.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionsMessageProcessor.java @@ -42,7 +42,7 @@ public TransactionsMessageProcessor( void processTransactionsMessage( final EthPeer peer, final TransactionsMessage transactionsMessage) { try { - LOG.debug("Received transactions message from {}", peer); + LOG.trace("Received transactions message from {}", peer); final Iterator readTransactions = transactionsMessage.transactions(Transaction::readFrom); From 67cedccc78ab45ff5819f06917ee4a54522a8acf Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Wed, 20 Feb 2019 13:04:14 +1000 Subject: [PATCH 4/6] refactoring to introduce deleteOnExit() for temp files (#920) --- .../dsl/node/ProcessPantheonNodeRunner.java | 1 + .../dsl/node/factory/PantheonNodeFactory.java | 1 + .../ethereum/p2p/NettyP2PNetworkTest.java | 7 +- .../internal/PeerDiscoveryControllerTest.java | 6 +- .../RecursivePeerRefreshStateTest.java | 6 +- .../AccountWhitelistControllerTest.java | 1 + .../NodeWhitelistControllerTest.java | 1 + ...PermissioningConfigurationBuilderTest.java | 38 ++++---- .../permissioning/WhitelistPersistorTest.java | 1 + .../pantheon/cli/PantheonCommandTest.java | 90 +++++++++---------- ...rmissioningConfigurationValidatorTest.java | 1 + 11 files changed, 81 insertions(+), 72 deletions(-) diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ProcessPantheonNodeRunner.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ProcessPantheonNodeRunner.java index b7b5655c50..d0dedc544a 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ProcessPantheonNodeRunner.java +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ProcessPantheonNodeRunner.java @@ -136,6 +136,7 @@ public void startNode(final PantheonNode node) { private Path createGenesisFile(final PantheonNode node, final EthNetworkConfig ethNetworkConfig) { try { Path genesisFile = Files.createTempFile(node.homeDirectory(), "genesis", ""); + genesisFile.toFile().deleteOnExit(); Files.write(genesisFile, ethNetworkConfig.getGenesisConfig().getBytes(UTF_8)); return genesisFile; } catch (IOException e) { diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonNodeFactory.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonNodeFactory.java index f2f94407c8..1fda4335bf 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonNodeFactory.java +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonNodeFactory.java @@ -223,6 +223,7 @@ public PantheonNode createNodeWithNodesWhitelist( PermissioningConfiguration.createDefault(); permissioningConfiguration.setNodeWhitelist(nodesWhitelist); File tempFile = createTempPermissioningConfigurationFile(); + tempFile.deleteOnExit(); permissioningConfiguration.setConfigurationFilePath(tempFile.getPath()); initPermissioningConfigurationFile( WhitelistPersistor.WHITELIST_TYPE.NODES, diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java index bf13091b63..f710ee2565 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java @@ -46,6 +46,7 @@ import java.net.InetAddress; import java.nio.file.Files; +import java.nio.file.Path; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -414,8 +415,10 @@ public void rejectIncomingConnectionFromNonWhitelistedPeer() throws Exception { final PeerBlacklist localBlacklist = new PeerBlacklist(); final PeerBlacklist remoteBlacklist = new PeerBlacklist(); final PermissioningConfiguration config = PermissioningConfiguration.createDefault(); - config.setConfigurationFilePath( - Files.createTempFile("test", "test").toAbsolutePath().toString()); + final Path tempFile = Files.createTempFile("test", "test"); + tempFile.toFile().deleteOnExit(); + config.setConfigurationFilePath(tempFile.toAbsolutePath().toString()); + final NodeWhitelistController localWhitelistController = new NodeWhitelistController(config, Collections.emptyList()); // turn on whitelisting by adding a different node NOT remote node diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java index 148db0eb44..1d47042576 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java @@ -51,6 +51,7 @@ import java.io.IOException; import java.net.URI; import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -1212,8 +1213,9 @@ private PeerDiscoveryController startPeerDiscoveryController( private PermissioningConfiguration permissioningConfigurationWithTempFile() throws IOException { final PermissioningConfiguration config = PermissioningConfiguration.createDefault(); - config.setConfigurationFilePath( - Files.createTempFile("test", "test").toAbsolutePath().toString()); + Path tempFile = Files.createTempFile("test", "test"); + tempFile.toFile().deleteOnExit(); + config.setConfigurationFilePath(tempFile.toAbsolutePath().toString()); return config; } diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/RecursivePeerRefreshStateTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/RecursivePeerRefreshStateTest.java index a9c56f2525..89958391d0 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/RecursivePeerRefreshStateTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/RecursivePeerRefreshStateTest.java @@ -32,6 +32,7 @@ import tech.pegasys.pantheon.util.bytes.BytesValue; import java.nio.file.Files; +import java.nio.file.Path; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -480,10 +481,11 @@ public void shouldNotBondWithNodesRejectedByWhitelist() throws Exception { final DiscoveryPeer peerA = new DiscoveryPeer(createId(1), "127.0.0.1", 1, 1); final DiscoveryPeer peerB = new DiscoveryPeer(createId(2), "127.0.0.2", 2, 2); + final Path tempFile = Files.createTempFile("test", "test"); + tempFile.toFile().deleteOnExit(); final PermissioningConfiguration permissioningConfiguration = PermissioningConfiguration.createDefault(); - permissioningConfiguration.setConfigurationFilePath( - Files.createTempFile("test", "test").toAbsolutePath().toString()); + permissioningConfiguration.setConfigurationFilePath(tempFile.toAbsolutePath().toString()); final NodeWhitelistController peerWhitelist = new NodeWhitelistController(permissioningConfiguration, Collections.emptyList()); diff --git a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/AccountWhitelistControllerTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/AccountWhitelistControllerTest.java index 0a7803e769..4823617af5 100644 --- a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/AccountWhitelistControllerTest.java +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/AccountWhitelistControllerTest.java @@ -234,6 +234,7 @@ public void accountThatDoesNotStartWith0xIsNotValid() { private Path createPermissionsFileWithAccount(final String account) throws IOException { final String nodePermissionsFileContent = "accounts-whitelist=[\"" + account + "\"]"; final Path permissionsFile = Files.createTempFile("account_permissions", ""); + permissionsFile.toFile().deleteOnExit(); Files.write(permissionsFile, nodePermissionsFileContent.getBytes(StandardCharsets.UTF_8)); return permissionsFile; } diff --git a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeWhitelistControllerTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeWhitelistControllerTest.java index c1bd99eece..0f2b97f6dd 100644 --- a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeWhitelistControllerTest.java +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeWhitelistControllerTest.java @@ -388,6 +388,7 @@ public void whenReloadingWhitelistAndNothingChangesShouldNotNotifyWhitelistModif private Path createPermissionsFileWithNode(final String node) throws IOException { final String nodePermissionsFileContent = "nodes-whitelist=[\"" + node + "\"]"; final Path permissionsFile = Files.createTempFile("node_permissions", ""); + permissionsFile.toFile().deleteOnExit(); Files.write(permissionsFile, nodePermissionsFileContent.getBytes(StandardCharsets.UTF_8)); return permissionsFile; } diff --git a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/PermissioningConfigurationBuilderTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/PermissioningConfigurationBuilderTest.java index cab95ffe5b..a6208535af 100644 --- a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/PermissioningConfigurationBuilderTest.java +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/PermissioningConfigurationBuilderTest.java @@ -12,9 +12,11 @@ */ package tech.pegasys.pantheon.ethereum.permissioning; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.catchThrowable; +import java.io.IOException; import java.net.URI; import java.net.URL; import java.nio.file.Files; @@ -53,8 +55,7 @@ public void permissioningConfig() throws Exception { final String uri2 = "enode://" + VALID_NODE_ID + "@192.169.0.9:4568"; final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_VALID); - final Path toml = Files.createTempFile("toml", ""); - Files.write(toml, Resources.toByteArray(configFile)); + final Path toml = createTempFile("toml", Resources.toByteArray(configFile)); PermissioningConfiguration permissioningConfiguration = permissioningConfig(toml); @@ -71,8 +72,7 @@ public void permissioningConfigWithOnlyNodeWhitelistSet() throws Exception { final String uri = "enode://" + VALID_NODE_ID + "@192.168.0.9:4567"; final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_NODE_WHITELIST_ONLY); - final Path toml = Files.createTempFile("toml", ""); - Files.write(toml, Resources.toByteArray(configFile)); + final Path toml = createTempFile("toml", Resources.toByteArray(configFile)); PermissioningConfiguration permissioningConfiguration = PermissioningConfigurationBuilder.permissioningConfiguration( @@ -86,8 +86,7 @@ public void permissioningConfigWithOnlyNodeWhitelistSet() throws Exception { @Test public void permissioningConfigWithOnlyAccountWhitelistSet() throws Exception { final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_ACCOUNT_WHITELIST_ONLY); - final Path toml = Files.createTempFile("toml", ""); - Files.write(toml, Resources.toByteArray(configFile)); + final Path toml = createTempFile("toml", Resources.toByteArray(configFile)); PermissioningConfiguration permissioningConfiguration = PermissioningConfigurationBuilder.permissioningConfiguration( @@ -102,8 +101,7 @@ public void permissioningConfigWithOnlyAccountWhitelistSet() throws Exception { @Test public void permissioningConfigWithInvalidAccount() throws Exception { final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_INVALID_ACCOUNT); - final Path toml = Files.createTempFile("toml", ""); - Files.write(toml, Resources.toByteArray(configFile)); + final Path toml = createTempFile("toml", Resources.toByteArray(configFile)); final Throwable thrown = catchThrowable(() -> permissioningConfig(toml)); @@ -115,8 +113,7 @@ public void permissioningConfigWithInvalidAccount() throws Exception { @Test public void permissioningConfigWithInvalidEnode() throws Exception { final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_INVALID_ENODE); - final Path toml = Files.createTempFile("toml", ""); - Files.write(toml, Resources.toByteArray(configFile)); + final Path toml = createTempFile("toml", Resources.toByteArray(configFile)); final Throwable thrown = catchThrowable(() -> permissioningConfig(toml)); @@ -128,8 +125,7 @@ public void permissioningConfigWithInvalidEnode() throws Exception { @Test public void permissioningConfigWithEmptyWhitelistMustNotError() throws Exception { final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_EMPTY_WHITELISTS); - final Path toml = Files.createTempFile("toml", ""); - Files.write(toml, Resources.toByteArray(configFile)); + final Path toml = createTempFile("toml", Resources.toByteArray(configFile)); PermissioningConfiguration permissioningConfiguration = permissioningConfig(toml); @@ -142,8 +138,7 @@ public void permissioningConfigWithEmptyWhitelistMustNotError() throws Exception @Test public void permissioningConfigWithAbsentWhitelistMustThrowException() throws Exception { final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_ABSENT_WHITELISTS); - final Path toml = Files.createTempFile("toml", ""); - Files.write(toml, Resources.toByteArray(configFile)); + final Path toml = createTempFile("toml", Resources.toByteArray(configFile)); final Throwable thrown = catchThrowable(() -> permissioningConfig(toml)); @@ -153,8 +148,7 @@ public void permissioningConfigWithAbsentWhitelistMustThrowException() throws Ex @Test public void permissioningConfigWithUnrecognizedKeyMustThrowException() throws Exception { final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_UNRECOGNIZED_KEY); - final Path toml = Files.createTempFile("toml", ""); - Files.write(toml, Resources.toByteArray(configFile)); + final Path toml = createTempFile("toml", Resources.toByteArray(configFile)); final Throwable thrown = catchThrowable(() -> permissioningConfig(toml)); @@ -167,7 +161,7 @@ public void permissioningConfigWithUnrecognizedKeyMustThrowException() throws Ex @Test public void permissioningConfigWithEmptyFileMustThrowException() throws Exception { // write an empty file - final Path toml = Files.createTempFile("toml", ""); + final Path toml = createTempFile("toml", "".getBytes(UTF_8)); final Throwable thrown = catchThrowable(() -> permissioningConfig(toml)); @@ -177,8 +171,7 @@ public void permissioningConfigWithEmptyFileMustThrowException() throws Exceptio @Test public void permissioningConfigFromFileMustSetFilePath() throws Exception { final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_VALID); - final Path toml = Files.createTempFile("toml", ""); - Files.write(toml, Resources.toByteArray(configFile)); + final Path toml = createTempFile("toml", Resources.toByteArray(configFile)); PermissioningConfiguration permissioningConfiguration = PermissioningConfigurationBuilder.permissioningConfigurationFromToml( @@ -213,4 +206,11 @@ private PermissioningConfiguration permissioningConfig(final Path toml) throws E return PermissioningConfigurationBuilder.permissioningConfiguration( toml.toAbsolutePath().toString(), true, true); } + + private Path createTempFile(final String filename, final byte[] contents) throws IOException { + final Path file = Files.createTempFile(filename, ""); + Files.write(file, contents); + file.toFile().deleteOnExit(); + return file; + } } diff --git a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistPersistorTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistPersistorTest.java index a3b2a0fdfa..9c948b8f3c 100644 --- a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistPersistorTest.java +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistPersistorTest.java @@ -43,6 +43,7 @@ public class WhitelistPersistorTest { public void setUp() throws IOException { List lines = Lists.newArrayList(nodesWhitelist, accountsWhitelist); tempFile = File.createTempFile("test", "test"); + tempFile.deleteOnExit(); Files.write(tempFile.toPath(), lines, StandardOpenOption.WRITE, StandardOpenOption.CREATE); whitelistPersistor = new WhitelistPersistor(tempFile.getAbsolutePath()); } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index a0f1aafeac..7949572c3a 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -48,7 +48,6 @@ import java.io.File; import java.io.IOException; -import java.io.Writer; import java.net.URI; import java.net.URL; import java.nio.file.Files; @@ -184,10 +183,11 @@ public void callingWithConfigOptionButNoConfigFileShouldDisplayHelp() { public void callingWithConfigOptionButNonExistingFileShouldDisplayHelp() throws IOException { assumeTrue(isFullInstantiation()); - final File tempConfigFile = temp.newFile("an-invalid-file-name-without-extension"); - parseCommand("--config-file", tempConfigFile.getPath()); + final Path tempConfigFilePath = createTempFile("an-invalid-file-name-without-extension", ""); + parseCommand("--config-file", tempConfigFilePath.toString()); - final String expectedOutputStart = "Unable to read TOML configuration file " + tempConfigFile; + final String expectedOutputStart = + "Unable to read TOML configuration file " + tempConfigFilePath; assertThat(commandErrorOutput.toString()).startsWith(expectedOutputStart); assertThat(commandOutput.toString()).isEmpty(); } @@ -209,20 +209,15 @@ public void callingWithConfigOptionButInvalidContentTomlFileShouldDisplayHelp() // We write a config file to prevent an invalid file in resource folder to raise errors in // code checks (CI + IDE) - final File tempConfigFile = temp.newFile("invalid_config.toml"); - try (final Writer fileWriter = Files.newBufferedWriter(tempConfigFile.toPath(), UTF_8)) { - - fileWriter.write("."); // an invalid toml content - fileWriter.flush(); + final Path tempConfigFile = createTempFile("invalid_config.toml", "."); - parseCommand("--config-file", tempConfigFile.getPath()); + parseCommand("--config-file", tempConfigFile.toString()); - final String expectedOutputStart = - "Invalid TOML configuration : Unexpected '.', expected a-z, A-Z, 0-9, ', \", a table key, " - + "a newline, or end-of-input (line 1, column 1)"; - assertThat(commandErrorOutput.toString()).startsWith(expectedOutputStart); - assertThat(commandOutput.toString()).isEmpty(); - } + final String expectedOutputStart = + "Invalid TOML configuration : Unexpected '.', expected a-z, A-Z, 0-9, ', \", a table key, " + + "a newline, or end-of-input (line 1, column 1)"; + assertThat(commandErrorOutput.toString()).startsWith(expectedOutputStart); + assertThat(commandOutput.toString()).isEmpty(); } @Test @@ -231,20 +226,14 @@ public void callingWithConfigOptionButInvalidValueTomlFileShouldDisplayHelp() th // We write a config file to prevent an invalid file in resource folder to raise errors in // code checks (CI + IDE) - final File tempConfigFile = temp.newFile("invalid_config.toml"); - try (final Writer fileWriter = Files.newBufferedWriter(tempConfigFile.toPath(), UTF_8)) { + final Path tempConfigFile = createTempFile("invalid_config.toml", "tester===========......."); + parseCommand("--config-file", tempConfigFile.toString()); - fileWriter.write("tester===========......."); // an invalid toml content - fileWriter.flush(); - - parseCommand("--config-file", tempConfigFile.getPath()); - - final String expectedOutputStart = - "Invalid TOML configuration : Unexpected '=', expected ', \", ''', \"\"\", a number, " - + "a boolean, a date/time, an array, or a table (line 1, column 8)"; - assertThat(commandErrorOutput.toString()).startsWith(expectedOutputStart); - assertThat(commandOutput.toString()).isEmpty(); - } + final String expectedOutputStart = + "Invalid TOML configuration : Unexpected '=', expected ', \", ''', \"\"\", a number, " + + "a boolean, a date/time, an array, or a table (line 1, column 8)"; + assertThat(commandErrorOutput.toString()).startsWith(expectedOutputStart); + assertThat(commandOutput.toString()).isEmpty(); } @Test @@ -256,8 +245,7 @@ public void overrideDefaultValuesIfKeyIsPresentInConfigFile() throws IOException final String updatedConfig = Resources.toString(configFile, UTF_8) .replace("~/genesis.json", escapeTomlString(genesisFile.toString())); - final Path toml = Files.createTempFile("toml", ""); - Files.write(toml, updatedConfig.getBytes(UTF_8)); + final Path toml = createTempFile("toml", updatedConfig.getBytes(UTF_8)); Collection expectedApis = asList(ETH, WEB3); @@ -341,10 +329,8 @@ public void permissionsEnabledWithNonexistentConfigFileMustError() { public void permissionsTomlFileWithNoPermissionsEnabledMustError() throws IOException { final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_TOML); - final Path permToml = Files.createTempFile("toml", ""); - Files.write(permToml, Resources.toByteArray(configFile)); + final Path permToml = createTempFile("toml", Resources.toByteArray(configFile)); parseCommand("--permissions-config-file", permToml.toString()); - permToml.toFile().deleteOnExit(); verify(mockRunnerBuilder).build(); @@ -353,7 +339,7 @@ public void permissionsTomlFileWithNoPermissionsEnabledMustError() throws IOExce } @Test - public void defaultPermissionsTomlFileWithNoPermissionsEnabledMustError() throws IOException { + public void defaultPermissionsTomlFileWithNoPermissionsEnabledMustError() { parseCommand("--p2p-enabled", "false"); verify(mockRunnerBuilder).build(); @@ -366,8 +352,7 @@ public void defaultPermissionsTomlFileWithNoPermissionsEnabledMustError() throws public void permissionsTomlPathMustUseOption() throws IOException { final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_TOML); - final Path permToml = Files.createTempFile("toml", ""); - Files.write(permToml, Resources.toByteArray(configFile)); + final Path permToml = createTempFile("toml", Resources.toByteArray(configFile)); parseCommand( "--permissions-accounts-enabled", "--permissions-config-file", permToml.toString()); @@ -394,8 +379,7 @@ public void tomlThatConfiguresEverythingExceptPermissioningToml() throws IOExcep // Load a TOML that configures literally everything (except permissioning TOML config) final URL configFile = Resources.getResource("everything_config.toml"); - final Path toml = Files.createTempFile("toml", ""); - Files.write(toml, Resources.toByteArray(configFile)); + final Path toml = createTempFile("toml", Resources.toByteArray(configFile)); // Parse it. final CommandLine.Model.CommandSpec spec = parseCommand("--config-file", toml.toString()); @@ -480,6 +464,7 @@ public void configOptionDisabledUnderDocker() { @Test public void nodekeyOptionMustBeUsed() throws Exception { final File file = new File("./specific/key"); + file.deleteOnExit(); parseCommand("--node-private-key-file", file.getPath()); @@ -501,6 +486,7 @@ public void nodekeyOptionDisabledUnderDocker() { assumeFalse(isFullInstantiation()); final File file = new File("./specific/key"); + file.deleteOnExit(); parseCommand("--node-private-key-file", file.getPath()); @@ -591,8 +577,6 @@ public void genesisAndNetworkMustNotBeUsedTogether() throws Exception { assumeTrue(isFullInstantiation()); final Path genesisFile = createFakeGenesisFile(GENESIS_VALID_JSON); - final ArgumentCaptor networkArg = - ArgumentCaptor.forClass(EthNetworkConfig.class); parseCommand("--genesis-file", genesisFile.toString(), "--network", "rinkeby"); @@ -1639,10 +1623,6 @@ public void metricsPrometheusJobMustBeUsed() { public void metricsAndMetricsPushMustNotBeUsedTogether() throws Exception { assumeTrue(isFullInstantiation()); - final Path genesisFile = createFakeGenesisFile(GENESIS_VALID_JSON); - final ArgumentCaptor networkArg = - ArgumentCaptor.forClass(EthNetworkConfig.class); - parseCommand("--metrics-enabled", "--metrics-push-enabled"); verifyZeroInteractions(mockRunnerBuilder); @@ -1880,15 +1860,16 @@ public void mustUseEnclaveUriAndOptions() throws IOException { } @Test - public void privacyOptionsRequiresServiceToBeEnabled() { + public void privacyOptionsRequiresServiceToBeEnabled() throws IOException { final File file = new File("./specific/public_key"); + file.deleteOnExit(); parseCommand( "--privacy-url", ENCLAVE_URI, "--privacy-public-key-file", - file.getPath(), + file.toString(), "--privacy-precompiled-address", String.valueOf(Byte.MAX_VALUE - 1)); @@ -1918,9 +1899,24 @@ public void mustVerifyPrivacyIsDisabled() throws IOException { private Path createFakeGenesisFile(final JsonObject jsonGenesis) throws IOException { final Path genesisFile = Files.createTempFile("genesisFile", ""); Files.write(genesisFile, encodeJsonGenesis(jsonGenesis).getBytes(UTF_8)); + genesisFile.toFile().deleteOnExit(); return genesisFile; } + private Path createTempFile(final String filename, final String contents) throws IOException { + final Path file = Files.createTempFile(filename, ""); + Files.write(file, contents.getBytes(UTF_8)); + file.toFile().deleteOnExit(); + return file; + } + + private Path createTempFile(final String filename, final byte[] contents) throws IOException { + final Path file = Files.createTempFile(filename, ""); + Files.write(file, contents); + file.toFile().deleteOnExit(); + return file; + } + private String encodeJsonGenesis(final JsonObject jsonGenesis) { return jsonGenesis.encodePrettily(); } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/util/PermissioningConfigurationValidatorTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/util/PermissioningConfigurationValidatorTest.java index da3e65afc1..b4cf39c40a 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/util/PermissioningConfigurationValidatorTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/util/PermissioningConfigurationValidatorTest.java @@ -58,6 +58,7 @@ public void nodesWhitelistOptionWhichDoesNotIncludeBootnodesMustError() throws E final URL configFile = Resources.getResource(PERMISSIONING_CONFIG); final Path toml = Files.createTempFile("toml", ""); + toml.toFile().deleteOnExit(); Files.write(toml, Resources.toByteArray(configFile)); PermissioningConfiguration permissioningConfiguration = From c02a5af8682c659b1e27426cf6fc0ed11e133ab6 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Wed, 20 Feb 2019 13:13:32 +1000 Subject: [PATCH 5/6] [PAN-2182] admin addpeers: error if node not whitelisted (#917) * added error for when addPeer is called for a non-whitelisted node --- .../internal/methods/AdminAddPeer.java | 4 +++ .../internal/response/JsonRpcError.java | 10 ++++--- .../internal/methods/AdminAddPeerTest.java | 28 +++++++++++++++++++ .../p2p/PeerNotWhitelistedException.java | 19 +++++++++++++ .../ethereum/p2p/netty/NettyP2PNetwork.java | 8 ++++++ 5 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/PeerNotWhitelistedException.java diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java index 28e6d71358..ea9e6d635a 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java @@ -20,6 +20,7 @@ import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcResponse; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse; import tech.pegasys.pantheon.ethereum.p2p.P2pDisabledException; +import tech.pegasys.pantheon.ethereum.p2p.PeerNotWhitelistedException; import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork; import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; @@ -58,6 +59,9 @@ public JsonRpcResponse response(final JsonRpcRequest req) { return new JsonRpcErrorResponse(req.getId(), JsonRpcError.PARSE_ERROR); } catch (final P2pDisabledException e) { return new JsonRpcErrorResponse(req.getId(), JsonRpcError.P2P_DISABLED); + } catch (final PeerNotWhitelistedException e) { + return new JsonRpcErrorResponse( + req.getId(), JsonRpcError.NON_WHITELISTED_NODE_CANNOT_BE_ADDED_AS_A_PEER); } catch (final Exception e) { LOG.error("Error processing request: " + req, e); throw e; diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcError.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcError.java index 99c4bb5ed1..551662a3a5 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcError.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcError.java @@ -25,7 +25,7 @@ public enum JsonRpcError { METHOD_NOT_FOUND(-32601, "Method not found"), INVALID_PARAMS(-32602, "Invalid params"), INTERNAL_ERROR(-32603, "Internal error"), - P2P_DISABLED(-32000, "P2P has been disabled. This functionality is not available."), + P2P_DISABLED(-32000, "P2P has been disabled. This functionality is not available"), // Filter & Subscription Errors FILTER_NOT_FOUND(-32000, "Filter not found"), @@ -44,8 +44,8 @@ public enum JsonRpcError { CHAIN_HEAD_WORLD_STATE_NOT_AVAILABLE(-32008, "Initial sync is still in progress"), // Miner failures - COINBASE_NOT_SET(-32010, "Coinbase not set. Unable to start mining without a coinbase."), - NO_HASHES_PER_SECOND(-32011, "No hashes being generated by the current node."), + COINBASE_NOT_SET(-32010, "Coinbase not set. Unable to start mining without a coinbase"), + NO_HASHES_PER_SECOND(-32011, "No hashes being generated by the current node"), // Wallet errors COINBASE_NOT_SPECIFIED(-32000, "Coinbase must be explicitly specified"), @@ -75,8 +75,10 @@ public enum JsonRpcError { "The permissioning whitelist configuration file is out of sync. The changes have been applied, but not persisted to disk"), WHITELIST_RELOAD_ERROR( -32000, - "Error reloading permissions file. Please use perm_getAccountsWhitelist and perm_getNodesWhitelist to review the current state of the whitelists."), + "Error reloading permissions file. Please use perm_getAccountsWhitelist and perm_getNodesWhitelist to review the current state of the whitelists"), PERMISSIONING_NOT_ENABLED(-32000, "Node/Account whitelisting has not been enabled"), + NON_WHITELISTED_NODE_CANNOT_BE_ADDED_AS_A_PEER( + -32000, "Cannot add a non-whitelisted node as a peer"), // Permissioning/Authorization errors UNAUTHORIZED(-40100, "Unauthorized"), diff --git a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeerTest.java b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeerTest.java index 1f48fc4944..bbe316380d 100644 --- a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeerTest.java +++ b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeerTest.java @@ -23,6 +23,7 @@ import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcResponse; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse; import tech.pegasys.pantheon.ethereum.p2p.P2pDisabledException; +import tech.pegasys.pantheon.ethereum.p2p.PeerNotWhitelistedException; import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork; import org.junit.Before; @@ -192,4 +193,31 @@ public void requestReturnsErrorWhenP2pDisabled() { assertThat(actualResponse).isEqualToComparingFieldByField(expectedResponse); } + + @Test + public void requestReturnsErrorWhenPeerNotWhitelisted() { + when(p2pNetwork.addMaintainConnectionPeer(any())) + .thenThrow(new PeerNotWhitelistedException("Cannot add peer that is not whitelisted")); + + final JsonRpcRequest request = + new JsonRpcRequest( + "2.0", + "admin_addPeer", + new String[] { + "enode://" + + "00000000000000000000000000000000" + + "00000000000000000000000000000000" + + "00000000000000000000000000000000" + + "00000000000000000000000000000000" + + "@127.0.0.1:30303" + }); + + final JsonRpcResponse expectedResponse = + new JsonRpcErrorResponse( + request.getId(), JsonRpcError.NON_WHITELISTED_NODE_CANNOT_BE_ADDED_AS_A_PEER); + + final JsonRpcResponse actualResponse = method.response(request); + + assertThat(actualResponse).isEqualToComparingFieldByField(expectedResponse); + } } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/PeerNotWhitelistedException.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/PeerNotWhitelistedException.java new file mode 100644 index 0000000000..7e558a9257 --- /dev/null +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/PeerNotWhitelistedException.java @@ -0,0 +1,19 @@ +/* + * 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.ethereum.p2p; + +public class PeerNotWhitelistedException extends RuntimeException { + public PeerNotWhitelistedException(final String message) { + super(message); + } +} diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java index 67276106ea..a76b657fa0 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java @@ -15,6 +15,7 @@ import static com.google.common.base.Preconditions.checkState; import tech.pegasys.pantheon.crypto.SECP256K1; +import tech.pegasys.pantheon.ethereum.p2p.PeerNotWhitelistedException; import tech.pegasys.pantheon.ethereum.p2p.api.DisconnectCallback; import tech.pegasys.pantheon.ethereum.p2p.api.Message; import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork; @@ -315,8 +316,15 @@ private boolean isPeerWhitelisted(final PeerConnection connection, final SocketC .orElse(true); } + private boolean isPeerWhitelisted(final Peer peer) { + return nodeWhitelistController.map(nwc -> nwc.isPermitted(peer.getEnodeURI())).orElse(true); + } + @Override public boolean addMaintainConnectionPeer(final Peer peer) { + if (!isPeerWhitelisted(peer)) { + throw new PeerNotWhitelistedException("Cannot add a peer that is not whitelisted"); + } final boolean added = peerMaintainConnectionList.add(peer); if (added) { connect(peer); From e5c930cd7c6c39639d06d948b39480ee77c1a36b Mon Sep 17 00:00:00 2001 From: tmohay Date: Wed, 20 Feb 2019 14:34:54 +1100 Subject: [PATCH 6/6] wip --- .../src/main/java/tech/pegasys/pantheon/util/BlockImporter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 cd6a6d282e..e4e539e9b5 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/util/BlockImporter.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/util/BlockImporter.java @@ -81,7 +81,7 @@ public BlockImporter.ImportResult importBlockchain( if (header.getNumber() == BlockHeader.GENESIS_BLOCK_NUMBER) { continue; } - if (header.getNumber() % 1 == 0) { + if (header.getNumber() % 100 == 0) { LOG.info("Import at block {}", header.getNumber()); } if (blockchain.contains(header.getHash())) {