diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContext.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContext.java index 6c38508869..f14fe07203 100644 --- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContext.java +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContext.java @@ -19,6 +19,7 @@ import tech.pegasys.pantheon.consensus.ibft.statemachine.IbftFinalState; import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.Block; +import tech.pegasys.pantheon.ethereum.core.BlockHeader; import java.util.ArrayList; import java.util.List; @@ -62,11 +63,13 @@ public class TestContext { return finalState.getMessageFactory(); } - public Block createBlockForProposal(final int round, final long timestamp) { - return finalState - .getBlockCreatorFactory() - .create(blockchain.getChainHeadHeader(), round) - .createBlock(timestamp); + public Block createBlockForProposal( + final BlockHeader parent, final int round, final long timestamp) { + return finalState.getBlockCreatorFactory().create(parent, round).createBlock(timestamp); + } + + public Block createBlockForProposalFromChainHead(final int round, final long timestamp) { + return createBlockForProposal(blockchain.getChainHeadHeader(), round, timestamp); } public RoundSpecificNodeRoles getRoundSpecificRoles(final ConsensusRoundIdentifier roundId) { diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestHelpers.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestHelpers.java index f568d31290..79b7006a52 100644 --- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestHelpers.java +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestHelpers.java @@ -37,7 +37,7 @@ import java.util.stream.Collectors; public class TestHelpers { public static SignedData createSignedCommentPayload( - final Block block, final KeyPair signingKeyPair, final ConsensusRoundIdentifier roundId) { + final ConsensusRoundIdentifier roundId, final Block block, final KeyPair signingKeyPair) { final IbftExtraData extraData = IbftExtraData.decode(block.getHeader().getExtraData()); diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/ValidatorPeer.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/ValidatorPeer.java index fe5799d758..49ff82e014 100644 --- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/ValidatorPeer.java +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/ValidatorPeer.java @@ -88,9 +88,13 @@ public class ValidatorPeer { return payload; } + public Signature getBlockSignature(final Hash digest) { + return SECP256K1.sign(digest, nodeKeys); + } + public SignedData injectCommit( final ConsensusRoundIdentifier rId, final Hash digest) { - final Signature commitSeal = SECP256K1.sign(digest, nodeKeys); + final Signature commitSeal = getBlockSignature(digest); final SignedData payload = messageFactory.createSignedCommitPayload(rId, digest, commitSeal); injectMessage(CommitMessageData.create(payload)); diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/FutureHeightTest.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/FutureHeightTest.java new file mode 100644 index 0000000000..1804c30908 --- /dev/null +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/FutureHeightTest.java @@ -0,0 +1,254 @@ +/* + * 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.tests; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.util.Lists.emptyList; +import static tech.pegasys.pantheon.consensus.ibft.support.MessageReceptionHelpers.assertPeersReceivedExactly; +import static tech.pegasys.pantheon.consensus.ibft.support.MessageReceptionHelpers.assertPeersReceivedNoMessages; +import static tech.pegasys.pantheon.consensus.ibft.support.TestHelpers.createSignedCommentPayload; + +import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; +import tech.pegasys.pantheon.consensus.ibft.IbftHelpers; +import tech.pegasys.pantheon.consensus.ibft.ibftevent.NewChainHead; +import tech.pegasys.pantheon.consensus.ibft.payload.CommitPayload; +import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory; +import tech.pegasys.pantheon.consensus.ibft.payload.PreparePayload; +import tech.pegasys.pantheon.consensus.ibft.payload.SignedData; +import tech.pegasys.pantheon.consensus.ibft.support.RoundSpecificNodeRoles; +import tech.pegasys.pantheon.consensus.ibft.support.TestContext; +import tech.pegasys.pantheon.consensus.ibft.support.TestContextFactory; +import tech.pegasys.pantheon.ethereum.core.Block; + +import java.time.Clock; +import java.time.Instant; +import java.time.ZoneId; +import java.util.stream.Collectors; + +import org.junit.Before; +import org.junit.Test; + +public class FutureHeightTest { + + private final long blockTimeStamp = 100; + private final Clock fixedClock = + Clock.fixed(Instant.ofEpochSecond(blockTimeStamp), ZoneId.systemDefault()); + + private final int NETWORK_SIZE = 5; + + // Context is configured such that a remote peer is responsible for proposing all block + private final TestContext context = + TestContextFactory.createTestEnvironment(NETWORK_SIZE, 0, fixedClock); + + private final ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(1, 0); + private final RoundSpecificNodeRoles roles = context.getRoundSpecificRoles(roundId); + + private final ConsensusRoundIdentifier futureHeightRoundId = new ConsensusRoundIdentifier(2, 0); + + private final MessageFactory localNodeMessageFactory = context.getLocalNodeMessageFactory(); + + @Before + public void setup() { + context.getController().start(); + } + + @Test + public void messagesForFutureHeightAreBufferedUntilChainHeightCatchesUp() { + final Block currentHeightBlock = context.createBlockForProposalFromChainHead(0, 30); + final Block signedCurrentHeightBlock = + IbftHelpers.createSealedBlock( + currentHeightBlock, + roles + .getAllPeers() + .stream() + .map(peer -> peer.getBlockSignature(currentHeightBlock.getHash())) + .collect(Collectors.toList())); + + final Block futureHeightBlock = + context.createBlockForProposal(signedCurrentHeightBlock.getHeader(), 0, 60); + + roles.getProposer().injectProposal(futureHeightRoundId, futureHeightBlock); + assertPeersReceivedNoMessages(roles.getAllPeers()); + + // Inject prepares and commits from all peers + roles + .getNonProposingPeers() + .forEach( + p -> { + p.injectPrepare(futureHeightRoundId, futureHeightBlock.getHash()); + p.injectCommit(futureHeightRoundId, futureHeightBlock.getHash()); + }); + assertPeersReceivedNoMessages(roles.getAllPeers()); + assertThat(context.getCurrentChainHeight()).isEqualTo(0); + + // Add block to chain, and notify system of its arrival. + context.getBlockchain().appendBlock(signedCurrentHeightBlock, emptyList()); + assertThat(context.getCurrentChainHeight()).isEqualTo(1); + context + .getController() + .handleNewBlockEvent(new NewChainHead(signedCurrentHeightBlock.getHeader())); + + final SignedData expectedPrepareMessage = + localNodeMessageFactory.createSignedPreparePayload( + futureHeightRoundId, futureHeightBlock.getHash()); + + final SignedData expectedCommitMessage = + createSignedCommentPayload( + futureHeightRoundId, futureHeightBlock, context.getLocalNodeParams().getNodeKeyPair()); + + assertPeersReceivedExactly(roles.getAllPeers(), expectedPrepareMessage, expectedCommitMessage); + assertThat(context.getCurrentChainHeight()).isEqualTo(2); + } + + @Test + public void messagesFromPreviousHeightAreDiscarded() { + final Block currentHeightBlock = context.createBlockForProposalFromChainHead(0, 30); + final Block signedCurrentHeightBlock = + IbftHelpers.createSealedBlock( + currentHeightBlock, + roles + .getAllPeers() + .stream() + .map(peer -> peer.getBlockSignature(currentHeightBlock.getHash())) + .collect(Collectors.toList())); + + roles.getProposer().injectProposal(roundId, currentHeightBlock); + roles.getNonProposingPeer(0).injectPrepare(roundId, currentHeightBlock.getHash()); + + final SignedData expectedPrepareMessage = + localNodeMessageFactory.createSignedPreparePayload(roundId, currentHeightBlock.getHash()); + + assertPeersReceivedExactly(roles.getAllPeers(), expectedPrepareMessage); + + // Add block to chain, and notify system of its arrival. + context.getBlockchain().appendBlock(signedCurrentHeightBlock, emptyList()); + assertThat(context.getCurrentChainHeight()).isEqualTo(1); + context + .getController() + .handleNewBlockEvent(new NewChainHead(signedCurrentHeightBlock.getHeader())); + + // Inject prepares and commits from all peers for the 'previous' round (i.e. the height + // from before the block arrived). + roles + .getNonProposingPeers() + .forEach( + p -> { + p.injectPrepare(roundId, currentHeightBlock.getHash()); + p.injectCommit(roundId, currentHeightBlock.getHash()); + }); + assertPeersReceivedNoMessages(roles.getAllPeers()); + assertThat(context.getCurrentChainHeight()).isEqualTo(1); + } + + @Test + public void multipleNewChainHeadEventsDoesNotRestartCurrentHeightManager() { + final Block currentHeightBlock = context.createBlockForProposalFromChainHead(0, 30); + + roles.getProposer().injectProposal(roundId, currentHeightBlock); + roles.getNonProposingPeer(0).injectPrepare(roundId, currentHeightBlock.getHash()); + + roles.getAllPeers().forEach(peer -> peer.clearReceivedMessages()); + + // inject a NewHeight FOR THE CURRENT HEIGHT + context + .getController() + .handleNewBlockEvent(new NewChainHead(context.getBlockchain().getChainHeadHeader())); + + // Should only require 1 more prepare to close it out + roles.getNonProposingPeer(1).injectPrepare(roundId, currentHeightBlock.getHash()); + + final SignedData expectedCommitMessage = + createSignedCommentPayload( + roundId, currentHeightBlock, context.getLocalNodeParams().getNodeKeyPair()); + assertPeersReceivedExactly(roles.getAllPeers(), expectedCommitMessage); + } + + @Test + public void correctMessagesAreExtractedFromFutureHeightBuffer() { + final Block currentHeightBlock = context.createBlockForProposalFromChainHead(0, 30); + final Block signedCurrentHeightBlock = + IbftHelpers.createSealedBlock( + currentHeightBlock, + roles + .getAllPeers() + .stream() + .map(peer -> peer.getBlockSignature(currentHeightBlock.getHash())) + .collect(Collectors.toList())); + + final Block nextHeightBlock = + context.createBlockForProposal(signedCurrentHeightBlock.getHeader(), 0, 60); + final Block signedNextHeightBlock = + IbftHelpers.createSealedBlock( + nextHeightBlock, + roles + .getAllPeers() + .stream() + .map(peer -> peer.getBlockSignature(nextHeightBlock.getHash())) + .collect(Collectors.toList())); + + final Block futureHeightBlock = + context.createBlockForProposal(signedNextHeightBlock.getHeader(), 0, 90); + + final ConsensusRoundIdentifier nextHeightRoundId = new ConsensusRoundIdentifier(2, 0); + final ConsensusRoundIdentifier futureHeightRoundId = new ConsensusRoundIdentifier(3, 0); + + // Inject prepares and commits from all peers into FutureHeight (2 height time) + roles + .getNonProposingPeers() + .forEach( + p -> { + p.injectPrepare(futureHeightRoundId, futureHeightBlock.getHash()); + p.injectCommit(futureHeightRoundId, futureHeightBlock.getHash()); + }); + + // Add the "interim" block to chain, and notify system of its arrival. + context.getBlockchain().appendBlock(signedCurrentHeightBlock, emptyList()); + assertThat(context.getCurrentChainHeight()).isEqualTo(1); + context + .getController() + .handleNewBlockEvent(new NewChainHead(signedCurrentHeightBlock.getHeader())); + + assertPeersReceivedNoMessages(roles.getAllPeers()); + roles.getProposer().injectProposal(nextHeightRoundId, nextHeightBlock); + + final SignedData expectedPrepareMessage = + localNodeMessageFactory.createSignedPreparePayload( + nextHeightRoundId, nextHeightBlock.getHash()); + + // Assert ONLY a prepare message was received, not any commits (i.e. futureHeightRoundId + // messages have not been used. + assertPeersReceivedExactly(roles.getAllPeers(), expectedPrepareMessage); + + roles.getProposer().injectProposal(futureHeightRoundId, futureHeightBlock); + + // Change to the FutureRound, and confirm prepare and commit msgs are sent + context.getBlockchain().appendBlock(signedNextHeightBlock, emptyList()); + assertThat(context.getCurrentChainHeight()).isEqualTo(2); + context + .getController() + .handleNewBlockEvent(new NewChainHead(signedNextHeightBlock.getHeader())); + + final SignedData expectedFuturePrepareMessage = + localNodeMessageFactory.createSignedPreparePayload( + futureHeightRoundId, futureHeightBlock.getHash()); + + final SignedData expectedCommitMessage = + createSignedCommentPayload( + futureHeightRoundId, futureHeightBlock, context.getLocalNodeParams().getNodeKeyPair()); + + // Assert ONLY a prepare message was received, not any commits (i.e. futureHeightRoundId + // messages have not been used. + assertPeersReceivedExactly( + roles.getAllPeers(), expectedCommitMessage, expectedFuturePrepareMessage); + } +} diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/FutureRoundTest.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/FutureRoundTest.java index 83685148f0..f2a46b8b72 100644 --- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/FutureRoundTest.java +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/FutureRoundTest.java @@ -64,7 +64,8 @@ public class FutureRoundTest { @Test public void messagesForFutureRoundAreNotActionedUntilRoundIsActive() { - final Block futureBlock = context.createBlockForProposal(futureRoundId.getRoundNumber(), 60); + final Block futureBlock = + context.createBlockForProposalFromChainHead(futureRoundId.getRoundNumber(), 60); final int quorum = IbftHelpers.calculateRequiredValidatorQuorum(NETWORK_SIZE); final ConsensusRoundIdentifier subsequentRoundId = new ConsensusRoundIdentifier(1, 6); final RoundSpecificNodeRoles subsequentRoles = context.getRoundSpecificRoles(subsequentRoundId); @@ -117,8 +118,10 @@ public class FutureRoundTest { @Test public void priorRoundsCannotBeCompletedAfterReceptionOfNewRound() { - final Block initialBlock = context.createBlockForProposal(roundId.getRoundNumber(), 30); - final Block futureBlock = context.createBlockForProposal(futureRoundId.getRoundNumber(), 60); + final Block initialBlock = + context.createBlockForProposalFromChainHead(roundId.getRoundNumber(), 30); + final Block futureBlock = + context.createBlockForProposalFromChainHead(futureRoundId.getRoundNumber(), 60); roles.getProposer().injectProposal(roundId, initialBlock); for (final ValidatorPeer peer : roles.getNonProposingPeers()) { diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/LocalNodeIsProposerTest.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/LocalNodeIsProposerTest.java index fea789e486..780f4bdc03 100644 --- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/LocalNodeIsProposerTest.java +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/LocalNodeIsProposerTest.java @@ -58,13 +58,13 @@ public class LocalNodeIsProposerTest { @Before public void setup() { - expectedProposedBlock = context.createBlockForProposal(0, blockTimeStamp); + expectedProposedBlock = context.createBlockForProposalFromChainHead(0, blockTimeStamp); expectedTxProposal = localNodeMessageFactory.createSignedProposalPayload(roundId, expectedProposedBlock); expectedTxCommit = createSignedCommentPayload( - expectedProposedBlock, context.getLocalNodeParams().getNodeKeyPair(), roundId); + roundId, expectedProposedBlock, context.getLocalNodeParams().getNodeKeyPair()); // Start the Controller, and trigger "block timer" to send proposal. context.getController().start(); diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/LocalNodeNotProposerTest.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/LocalNodeNotProposerTest.java index 25891971c6..a519e74597 100644 --- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/LocalNodeNotProposerTest.java +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/LocalNodeNotProposerTest.java @@ -15,10 +15,9 @@ package tech.pegasys.pantheon.consensus.ibft.tests; import static org.assertj.core.api.Assertions.assertThat; import static tech.pegasys.pantheon.consensus.ibft.support.MessageReceptionHelpers.assertPeersReceivedExactly; import static tech.pegasys.pantheon.consensus.ibft.support.MessageReceptionHelpers.assertPeersReceivedNoMessages; +import static tech.pegasys.pantheon.consensus.ibft.support.TestHelpers.createSignedCommentPayload; import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; -import tech.pegasys.pantheon.consensus.ibft.IbftBlockHashing; -import tech.pegasys.pantheon.consensus.ibft.IbftExtraData; import tech.pegasys.pantheon.consensus.ibft.payload.CommitPayload; import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory; import tech.pegasys.pantheon.consensus.ibft.payload.PreparePayload; @@ -27,8 +26,6 @@ import tech.pegasys.pantheon.consensus.ibft.support.RoundSpecificNodeRoles; import tech.pegasys.pantheon.consensus.ibft.support.TestContext; import tech.pegasys.pantheon.consensus.ibft.support.TestContextFactory; import tech.pegasys.pantheon.consensus.ibft.support.ValidatorPeer; -import tech.pegasys.pantheon.crypto.SECP256K1; -import tech.pegasys.pantheon.crypto.SECP256K1.Signature; import tech.pegasys.pantheon.ethereum.core.Block; import org.junit.Before; @@ -44,7 +41,7 @@ public class LocalNodeNotProposerTest { private final MessageFactory localNodeMessageFactory = context.getLocalNodeMessageFactory(); - private final Block blockToPropose = context.createBlockForProposal(0, 15); + private final Block blockToPropose = context.createBlockForProposalFromChainHead(0, 15); private SignedData expectedTxPrepare; private SignedData expectedTxCommit; @@ -54,16 +51,9 @@ public class LocalNodeNotProposerTest { expectedTxPrepare = localNodeMessageFactory.createSignedPreparePayload(roundId, blockToPropose.getHash()); - final IbftExtraData extraData = IbftExtraData.decode(blockToPropose.getHeader().getExtraData()); - final Signature commitSeal = - SECP256K1.sign( - IbftBlockHashing.calculateDataHashForCommittedSeal( - blockToPropose.getHeader(), extraData), - context.getLocalNodeParams().getNodeKeyPair()); - expectedTxCommit = - localNodeMessageFactory.createSignedCommitPayload( - roundId, blockToPropose.getHash(), commitSeal); + createSignedCommentPayload( + roundId, blockToPropose, context.getLocalNodeParams().getNodeKeyPair()); context.getController().start(); } diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/ReceivedNewRoundTest.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/ReceivedNewRoundTest.java index 1ad8cb9c5f..bdfa96186a 100644 --- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/ReceivedNewRoundTest.java +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/ReceivedNewRoundTest.java @@ -58,7 +58,8 @@ public class ReceivedNewRoundTest { @Test public void newRoundMessageWithEmptyPrepareCertificatesOfferNewBlock() { final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1); - final Block blockToPropose = context.createBlockForProposal(nextRoundId.getRoundNumber(), 15); + final Block blockToPropose = + context.createBlockForProposalFromChainHead(nextRoundId.getRoundNumber(), 15); final ConsensusRoundIdentifier targetRound = new ConsensusRoundIdentifier(1, 1); final List> roundChanges = @@ -84,7 +85,8 @@ public class ReceivedNewRoundTest { @Test public void newRoundMessageFromIllegalSenderIsDiscardedAndNoPrepareForNewRoundIsSent() { final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1); - final Block blockToPropose = context.createBlockForProposal(nextRoundId.getRoundNumber(), 15); + final Block blockToPropose = + context.createBlockForProposalFromChainHead(nextRoundId.getRoundNumber(), 15); final List> roundChanges = roles @@ -108,8 +110,8 @@ public class ReceivedNewRoundTest { @Test public void newRoundWithPrepareCertificateResultsInNewRoundStartingWithExpectedBlock() { - final Block initialBlock = context.createBlockForProposal(0, 15); - final Block reproposedBlock = context.createBlockForProposal(1, 15); + final Block initialBlock = context.createBlockForProposalFromChainHead(0, 15); + final Block reproposedBlock = context.createBlockForProposalFromChainHead(1, 15); final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1); final PreparedCertificate preparedCertificate = @@ -163,7 +165,8 @@ public class ReceivedNewRoundTest { final SignedData proposal = interimRoundProposer .getMessageFactory() - .createSignedProposalPayload(interimRound, context.createBlockForProposal(1, 30)); + .createSignedProposalPayload( + interimRound, context.createBlockForProposalFromChainHead(1, 30)); interimRoundProposer.injectNewRound( interimRound, new RoundChangeCertificate(roundChangePayloads), proposal); @@ -173,8 +176,8 @@ public class ReceivedNewRoundTest { @Test public void receiveRoundStateIsNotLostIfASecondNewRoundMessageIsReceivedForCurrentRound() { - final Block initialBlock = context.createBlockForProposal(0, 15); - final Block reproposedBlock = context.createBlockForProposal(1, 15); + final Block initialBlock = context.createBlockForProposalFromChainHead(0, 15); + final Block reproposedBlock = context.createBlockForProposalFromChainHead(1, 15); final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1); final PreparedCertificate preparedCertificate = @@ -224,7 +227,7 @@ public class ReceivedNewRoundTest { final SignedData expectedCommit = TestHelpers.createSignedCommentPayload( - reproposedBlock, context.getLocalNodeParams().getNodeKeyPair(), nextRoundId); + nextRoundId, reproposedBlock, context.getLocalNodeParams().getNodeKeyPair()); assertPeersReceivedExactly(nextRoles.getAllPeers(), expectedCommit); } diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/RoundChangeTest.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/RoundChangeTest.java index 2978852b50..5622966cea 100644 --- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/RoundChangeTest.java +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/RoundChangeTest.java @@ -61,7 +61,7 @@ public class RoundChangeTest { private final MessageFactory localNodeMessageFactory = context.getLocalNodeMessageFactory(); - private final Block blockToPropose = context.createBlockForProposal(0, 15); + private final Block blockToPropose = context.createBlockForProposalFromChainHead(0, 15); @Before public void setup() { @@ -141,7 +141,7 @@ public class RoundChangeTest { // Note: Round-4 is the next round for which the local node is Proposer final ConsensusRoundIdentifier targetRound = new ConsensusRoundIdentifier(1, 4); final Block locallyProposedBlock = - context.createBlockForProposal(targetRound.getRoundNumber(), blockTimeStamp); + context.createBlockForProposalFromChainHead(targetRound.getRoundNumber(), blockTimeStamp); final SignedData rc1 = roles.getNonProposingPeer(0).injectRoundChange(targetRound, empty()); @@ -169,13 +169,13 @@ public class RoundChangeTest { createValidPreparedCertificate( context, new ConsensusRoundIdentifier(1, 1), - context.createBlockForProposal(1, ARBITRARY_BLOCKTIME / 2)); + context.createBlockForProposalFromChainHead(1, ARBITRARY_BLOCKTIME / 2)); final PreparedCertificate bestPrepCert = createValidPreparedCertificate( context, new ConsensusRoundIdentifier(1, 2), - context.createBlockForProposal(2, ARBITRARY_BLOCKTIME)); + context.createBlockForProposalFromChainHead(2, ARBITRARY_BLOCKTIME)); final ConsensusRoundIdentifier targetRound = new ConsensusRoundIdentifier(1, 4); @@ -197,7 +197,8 @@ public class RoundChangeTest { // Expected to use the block with "ARBITRARY_BLOCKTIME" (i.e. latter block) but with the target // round number. final Block expectedBlockToPropose = - context.createBlockForProposal(targetRound.getRoundNumber(), ARBITRARY_BLOCKTIME); + context.createBlockForProposalFromChainHead( + targetRound.getRoundNumber(), ARBITRARY_BLOCKTIME); final SignedData expectedNewRound = localNodeMessageFactory.createSignedNewRoundPayload( @@ -225,7 +226,7 @@ public class RoundChangeTest { } final Block locallyProposedBlock = - context.createBlockForProposal(futureRound.getRoundNumber(), blockTimeStamp); + context.createBlockForProposalFromChainHead(futureRound.getRoundNumber(), blockTimeStamp); final SignedData expectedNewRound = localNodeMessageFactory.createSignedNewRoundPayload( @@ -260,7 +261,7 @@ public class RoundChangeTest { createValidPreparedCertificate( context, new ConsensusRoundIdentifier(1, 2), - context.createBlockForProposal(2, ARBITRARY_BLOCKTIME)); + context.createBlockForProposalFromChainHead(2, ARBITRARY_BLOCKTIME)); List> roundChangeMessages = Lists.newArrayList(); // Create a roundChange containing a PreparedCertificate @@ -278,7 +279,8 @@ public class RoundChangeTest { .collect(Collectors.toList())); final Block expectedBlockToPropose = - context.createBlockForProposal(targetRound.getRoundNumber(), ARBITRARY_BLOCKTIME); + context.createBlockForProposalFromChainHead( + targetRound.getRoundNumber(), ARBITRARY_BLOCKTIME); final SignedData expectedNewRound = localNodeMessageFactory.createSignedNewRoundPayload( 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 733eb2419c..b553bf5762 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 @@ -244,6 +244,10 @@ public class IbftBlockHeightManager { return currentRound.getRoundIdentifier().getSequenceNumber(); } + public BlockHeader getParentBlockHeader() { + return parentHeader; + } + private MessageAge determineAgeOfPayload(final Payload payload) { final int messageRoundNumber = payload.getRoundIdentifier().getRoundNumber(); final int currentRoundNumber = currentRound.getRoundIdentifier().getRoundNumber(); diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java index cb1edef0a5..159fdd2374 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java @@ -44,6 +44,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; public class IbftController { + private static final Logger LOG = LogManager.getLogger(); private final Blockchain blockchain; private final IbftFinalState ibftFinalState; @@ -141,7 +142,22 @@ public class IbftController { } public void handleNewBlockEvent(final NewChainHead newChainHead) { - startNewHeightManager(newChainHead.getNewChainHeadHeader()); + final BlockHeader newBlockHeader = newChainHead.getNewChainHeadHeader(); + final BlockHeader currentMiningParent = currentHeightManager.getParentBlockHeader(); + if (newBlockHeader.getNumber() < currentMiningParent.getNumber()) { + LOG.info("Discarding NewChainHead event, was for previous block height."); + return; + } + + if (newBlockHeader.getNumber() == currentMiningParent.getNumber()) { + if (newBlockHeader.getHash().equals(currentMiningParent.getHash())) { + LOG.info("Discarding duplicate NewChainHead event."); + } else { + LOG.error("Subsequent NewChainHead event at same block height indicates chain fork."); + } + return; + } + startNewHeightManager(newBlockHeader); } public void handleBlockTimerExpiry(final BlockTimerExpiry blockTimerExpiry) { diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftControllerTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftControllerTest.java index dae3cc30a3..ada2f0fbe4 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftControllerTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftControllerTest.java @@ -17,6 +17,7 @@ import static org.assertj.core.util.Lists.newArrayList; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -42,6 +43,7 @@ import tech.pegasys.pantheon.consensus.ibft.payload.SignedData; import tech.pegasys.pantheon.ethereum.chain.Blockchain; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; +import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.p2p.api.Message; import tech.pegasys.pantheon.ethereum.p2p.wire.DefaultMessage; @@ -62,7 +64,8 @@ public class IbftControllerTest { @Mock private Blockchain blockChain; @Mock private IbftFinalState ibftFinalState; @Mock private IbftBlockHeightManagerFactory blockHeightManagerFactory; - @Mock private BlockHeader blockHeader; + @Mock private BlockHeader chainHeadBlockHeader; + @Mock private BlockHeader nextBlock; @Mock private IbftBlockHeightManager blockHeightManager; @Mock private SignedData signedProposal; @@ -100,19 +103,26 @@ public class IbftControllerTest { @Before public void setup() { - when(blockChain.getChainHeadHeader()).thenReturn(blockHeader); - when(blockHeightManagerFactory.create(blockHeader)).thenReturn(blockHeightManager); + when(blockChain.getChainHeadHeader()).thenReturn(chainHeadBlockHeader); + when(blockHeightManagerFactory.create(any())).thenReturn(blockHeightManager); when(ibftFinalState.getValidators()).thenReturn(ImmutableList.of(validator)); ibftController = new IbftController( blockChain, ibftFinalState, blockHeightManagerFactory, futureMessages, ibftGossip); + + when(chainHeadBlockHeader.getNumber()).thenReturn(1L); + when(chainHeadBlockHeader.getHash()).thenReturn(Hash.ZERO); + + when(blockHeightManager.getParentBlockHeader()).thenReturn(chainHeadBlockHeader); + + when(nextBlock.getNumber()).thenReturn(2L); } @Test public void createsNewBlockHeightManagerWhenStarted() { ibftController.start(); assertThat(futureMessages).isEmpty(); - verify(blockHeightManagerFactory).create(blockHeader); + verify(blockHeightManagerFactory).create(chainHeadBlockHeader); } @Test @@ -134,7 +144,7 @@ public class IbftControllerTest { ibftController.start(); assertThat(futureMessages.keySet()).hasSize(1); assertThat(futureMessages.get(3L)).isEqualTo(height3Msgs); - verify(blockHeightManagerFactory).create(blockHeader); + verify(blockHeightManagerFactory).create(chainHeadBlockHeader); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verify(blockHeightManager, never()).handleProposalPayload(signedProposal); @@ -161,12 +171,13 @@ public class IbftControllerTest { prepareMessage, proposalMessage, commitMessage, roundChangeMessage, newRoundMessage)); when(blockHeightManager.getChainHeight()).thenReturn(2L); - final NewChainHead newChainHead = new NewChainHead(blockHeader); + ibftController.start(); + final NewChainHead newChainHead = new NewChainHead(nextBlock); ibftController.handleNewBlockEvent(newChainHead); - verify(blockHeightManagerFactory).create(blockHeader); + verify(blockHeightManagerFactory).create(nextBlock); verify(blockHeightManager, atLeastOnce()).getChainHeight(); - verify(blockHeightManager).start(); + verify(blockHeightManager, times(2)).start(); // once at beginning, and again on newChainHead. verify(blockHeightManager).handleProposalPayload(signedProposal); verify(ibftGossip).gossipMessage(proposalMessage); verify(blockHeightManager).handlePreparePayload(signedPrepare); @@ -179,6 +190,25 @@ public class IbftControllerTest { verify(ibftGossip).gossipMessage(newRoundMessage); } + @Test + public void newBlockForCurrentOrPreviousHeightTriggersNoChange() { + ibftController.start(); + + long chainHeadHeight = chainHeadBlockHeader.getNumber(); + when(nextBlock.getNumber()).thenReturn(chainHeadHeight); + when(nextBlock.getHash()).thenReturn(Hash.ZERO); + final NewChainHead sameHeightBlock = new NewChainHead(nextBlock); + ibftController.handleNewBlockEvent(sameHeightBlock); + verify(blockHeightManagerFactory, times(1)).create(any()); // initial creation + verify(blockHeightManager, times(1)).start(); // the initial call at start of test. + + when(nextBlock.getNumber()).thenReturn(chainHeadHeight - 1); + final NewChainHead priorBlock = new NewChainHead(nextBlock); + ibftController.handleNewBlockEvent(priorBlock); + verify(blockHeightManagerFactory, times(1)).create(any()); + verify(blockHeightManager, times(1)).start(); + } + @Test public void handlesRoundExpiry() { final RoundExpiry roundExpiry = new RoundExpiry(roundIdentifier);