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 b98d464a46..733eb2419c 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 @@ -236,7 +236,7 @@ public class IbftBlockHeightManager { if (messageAge == FUTURE_ROUND) { startNewRound(payload.getRoundIdentifier().getRoundNumber()); } - currentRound.handleProposalMessage(payload.getProposalPayload()); + currentRound.handleProposalFromNewRound(signedPayload); } } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRound.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRound.java index 96f5d81ca5..b9d7cc4d85 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRound.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRound.java @@ -23,6 +23,7 @@ import tech.pegasys.pantheon.consensus.ibft.blockcreation.IbftBlockCreator; import tech.pegasys.pantheon.consensus.ibft.network.IbftMessageTransmitter; import tech.pegasys.pantheon.consensus.ibft.payload.CommitPayload; import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory; +import tech.pegasys.pantheon.consensus.ibft.payload.NewRoundPayload; import tech.pegasys.pantheon.consensus.ibft.payload.PreparePayload; import tech.pegasys.pantheon.consensus.ibft.payload.PreparedCertificate; import tech.pegasys.pantheon.consensus.ibft.payload.ProposalPayload; @@ -136,8 +137,26 @@ public class IbftRound { public void handleProposalMessage(final SignedData msg) { LOG.info("Handling a Proposal message."); + + if (getRoundIdentifier().getRoundNumber() != 0) { + LOG.info("Illegally received a Proposal message when not in Round 0."); + return; + } + actionReceivedProposal(msg); + } + + public void handleProposalFromNewRound(final SignedData msg) { + LOG.info("Handling a New Round Proposal."); + + if (getRoundIdentifier().getRoundNumber() == 0) { + LOG.info("Illegally received a NewRound message when in Round 0."); + return; + } + actionReceivedProposal(msg.getPayload().getProposalPayload()); + } + + private void actionReceivedProposal(final SignedData msg) { final Block block = msg.getPayload().getBlock(); - final boolean wasCommitted = roundState.isCommitted(); if (updateStateWithProposedBlock(msg)) { LOG.info("Sending prepare message."); @@ -147,10 +166,6 @@ public class IbftRound { roundState.getRoundIdentifier(), block.getHash()); peerIsPrepared(localPrepareMessage); } - - if (wasCommitted != roundState.isCommitted()) { - importBlockToChain(); - } } public void handlePrepareMessage(final SignedData msg) { @@ -169,6 +184,7 @@ public class IbftRound { private boolean updateStateWithProposedBlock(final SignedData msg) { final boolean wasPrepared = roundState.isPrepared(); + final boolean wasCommitted = roundState.isCommitted(); final boolean blockAccepted = roundState.setProposedBlock(msg); if (blockAccepted) { // There are times handling a proposed block is enough to enter prepared. @@ -177,6 +193,10 @@ public class IbftRound { final Block block = roundState.getProposedBlock().get(); transmitter.multicastCommit(getRoundIdentifier(), block.getHash(), createCommitSeal(block)); } + if (wasCommitted != roundState.isCommitted()) { + importBlockToChain(); + } + final SignedData localCommitMessage = messageFactory.createSignedCommitPayload( roundState.getRoundIdentifier(), 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 1b21e5f4be..5b523674ec 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 @@ -67,7 +67,7 @@ import org.mockito.junit.MockitoJUnitRunner; public class IbftRoundTest { private final KeyPair localNodeKeys = KeyPair.generate(); - private final ConsensusRoundIdentifier roundIdentifier = new ConsensusRoundIdentifier(1, 1); + private final ConsensusRoundIdentifier roundIdentifier = new ConsensusRoundIdentifier(1, 0); private final MessageFactory messageFactory = new MessageFactory(localNodeKeys); private final Subscribers subscribers = new Subscribers<>(); private ProtocolContext protocolContext; @@ -375,4 +375,55 @@ public class IbftRoundTest { round.createAndSendProposalMessage(15); verify(minedBlockObserver).blockMined(any()); } + + @Test + public void blockIsOnlyImportedOnceWhenCommitsAreReceivedBeforeProposal() { + final ConsensusRoundIdentifier roundIdentifier = new ConsensusRoundIdentifier(1, 0); + final int QUORUM_SIZE = 2; + final RoundState roundState = new RoundState(roundIdentifier, QUORUM_SIZE, messageValidator); + final IbftRound round = + new IbftRound( + roundState, + blockCreator, + protocolContext, + blockImporter, + subscribers, + localNodeKeys, + messageFactory, + transmitter); + + round.handleCommitMessage( + messageFactory.createSignedCommitPayload( + roundIdentifier, proposedBlock.getHash(), remoteCommitSeal)); + + round.handleProposalMessage( + messageFactory.createSignedProposalPayload(roundIdentifier, proposedBlock)); + + verify(blockImporter, times(1)).importBlock(any(), any(), any()); + } + + @Test + public void blockIsImportedOnlyOnceIfQuorumCommitsAreReceivedPriorToProposal() { + final int QUORUM_SIZE = 1; + final RoundState roundState = new RoundState(roundIdentifier, QUORUM_SIZE, messageValidator); + final IbftRound round = + new IbftRound( + roundState, + blockCreator, + protocolContext, + blockImporter, + subscribers, + localNodeKeys, + messageFactory, + transmitter); + + round.handleCommitMessage( + messageFactory.createSignedCommitPayload( + roundIdentifier, proposedBlock.getHash(), remoteCommitSeal)); + + round.handleProposalMessage( + messageFactory.createSignedProposalPayload(roundIdentifier, proposedBlock)); + + verify(blockImporter, times(1)).importBlock(any(), any(), any()); + } }