NewRoundMessageValidator creates RoundChangeValidator with correct value (#518)

consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftHelpers.java

@@ -34,6 +34,10 @@ public class IbftHelpers {
     return Util.fastDivCeiling(2 * validatorCount, 3);
   }
 
+  public static long prepareMessageCountForQuorum(final long quorum) {
+    return quorum - 1;
+  }
+
   public static Block createSealedBlock(
       final Block block, final Collection<Signature> commitSeals) {
     final BlockHeader initialHeader = block.getHeader();

consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeManager.java

@@ -12,6 +12,8 @@
  */
 package tech.pegasys.pantheon.consensus.ibft.statemachine;
 
+import static tech.pegasys.pantheon.consensus.ibft.IbftHelpers.prepareMessageCountForQuorum;
+
 import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
 import tech.pegasys.pantheon.consensus.ibft.IbftHelpers;
 import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangeCertificate;
@@ -89,7 +91,10 @@ public class RoundChangeManager {
     this.quorumSize = IbftHelpers.calculateRequiredValidatorQuorum(validators.size());
     this.roundChangeMessageValidator =
         new RoundChangeMessageValidator(
-            messageValidityFactory, validators, quorumSize - 1, sequenceNumber);
+            messageValidityFactory,
+            validators,
+            prepareMessageCountForQuorum(quorumSize),
+            sequenceNumber);
   }
 
   /**

consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundState.java

@@ -12,6 +12,8 @@
  */
 package tech.pegasys.pantheon.consensus.ibft.statemachine;
 
+import static tech.pegasys.pantheon.consensus.ibft.IbftHelpers.prepareMessageCountForQuorum;
+
 import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
 import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.CommitPayload;
 import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparePayload;
@@ -92,7 +94,9 @@ public class RoundState {
   private void updateState() {
     // NOTE: The quorumSize for Prepare messages is 1 less than the quorum size as the proposer
     // does not supply a prepare message
-    prepared = (preparePayloads.size() >= (quorumSize - 1)) && proposalMessage.isPresent();
+    prepared =
+        (preparePayloads.size() >= prepareMessageCountForQuorum(quorumSize))
+            && proposalMessage.isPresent();
     committed = (commitPayloads.size() >= quorumSize) && proposalMessage.isPresent();
   }
 

consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/MessageValidatorFactory.java

@@ -12,6 +12,8 @@
  */
 package tech.pegasys.pantheon.consensus.ibft.validation;
 
+import static tech.pegasys.pantheon.consensus.ibft.IbftHelpers.prepareMessageCountForQuorum;
+
 import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
 import tech.pegasys.pantheon.consensus.ibft.IbftContext;
 import tech.pegasys.pantheon.consensus.ibft.IbftHelpers;
@@ -56,7 +58,8 @@ public class MessageValidatorFactory {
     return new RoundChangeMessageValidator(
         roundIdentifier -> createMessageValidator(roundIdentifier, parentHeader),
         validators,
-        IbftHelpers.calculateRequiredValidatorQuorum(validators.size()),
+        prepareMessageCountForQuorum(
+            IbftHelpers.calculateRequiredValidatorQuorum(validators.size())),
         parentHeader.getNumber() + 1);
   }
 

consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidator.java

@@ -13,6 +13,7 @@
 package tech.pegasys.pantheon.consensus.ibft.validation;
 
 import static tech.pegasys.pantheon.consensus.ibft.IbftHelpers.findLatestPreparedCertificate;
+import static tech.pegasys.pantheon.consensus.ibft.IbftHelpers.prepareMessageCountForQuorum;
 
 import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
 import tech.pegasys.pantheon.consensus.ibft.blockcreation.ProposerSelector;
@@ -123,7 +124,10 @@ public class NewRoundMessageValidator {
         roundChangeCert.getRoundChangePayloads()) {
       final RoundChangeMessageValidator roundChangeValidator =
           new RoundChangeMessageValidator(
-              messageValidatorFactory, validators, quorumSize, chainHeight);
+              messageValidatorFactory,
+              validators,
+              prepareMessageCountForQuorum(quorumSize),
+              chainHeight);
 
       if (!roundChangeValidator.validateMessage(roundChangeMsg)) {
         LOG.info("Invalid NewRound message, embedded RoundChange message failed validation.");

consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidatorTest.java

@@ -128,7 +128,7 @@ public class NewRoundMessageValidatorTest {
   }
 
   @Test
-  public void newRoundTargettingRoundZeroFails() {
+  public void newRoundTargetingRoundZeroFails() {
     msgBuilder.setRoundChangeIdentifier(
         new ConsensusRoundIdentifier(roundIdentifier.getSequenceNumber(), 0));
 
@@ -205,7 +205,7 @@ public class NewRoundMessageValidatorTest {
   public void roundChangeMessagesDoNotAllTargetRoundOfNewRoundMsgFails() {
     final ConsensusRoundIdentifier prevRound = TestHelpers.createFrom(roundIdentifier, 0, -1);
 
-    RoundChangeCertificate.Builder roundChangeBuilder = new RoundChangeCertificate.Builder();
+    final RoundChangeCertificate.Builder roundChangeBuilder = new RoundChangeCertificate.Builder();
     roundChangeBuilder.appendRoundChangeMessage(
         proposerMessageFactory.createSignedRoundChangePayload(roundIdentifier, Optional.empty()));
     roundChangeBuilder.appendRoundChangeMessage(
@@ -222,7 +222,7 @@ public class NewRoundMessageValidatorTest {
   public void invalidEmbeddedRoundChangeMessageFails() {
     final ConsensusRoundIdentifier prevRound = TestHelpers.createFrom(roundIdentifier, 0, -1);
 
-    RoundChangeCertificate.Builder roundChangeBuilder = new RoundChangeCertificate.Builder();
+    final RoundChangeCertificate.Builder roundChangeBuilder = new RoundChangeCertificate.Builder();
     roundChangeBuilder.appendRoundChangeMessage(
         proposerMessageFactory.createSignedRoundChangePayload(
             roundIdentifier,
@@ -260,8 +260,6 @@ public class NewRoundMessageValidatorTest {
                 differentProposal,
                 Lists.newArrayList(
                     validatorMessageFactory.createSignedPreparePayload(
-                        roundIdentifier, proposedBlock.getHash()),
-                    otherValidatorMessageFactory.createSignedPreparePayload(
                         roundIdentifier, proposedBlock.getHash()))));
 
     // An earlier PrepareCert is added to ensure the path to find the latest PrepareCert
@@ -277,8 +275,6 @@ public class NewRoundMessageValidatorTest {
                 earlierProposal,
                 Lists.newArrayList(
                     validatorMessageFactory.createSignedPreparePayload(
-                        earlierPreparedRound, proposedBlock.getHash()),
-                    otherValidatorMessageFactory.createSignedPreparePayload(
                         earlierPreparedRound, proposedBlock.getHash()))));
 
     final SignedData<NewRoundPayload> msg =
@@ -287,9 +283,7 @@ public class NewRoundMessageValidatorTest {
             new RoundChangeCertificate(
                 Lists.newArrayList(
                     proposerMessageFactory.createSignedRoundChangePayload(
-                        roundIdentifier, earlierPreparedCert),
-                    proposerMessageFactory.createSignedRoundChangePayload(
-                        roundIdentifier, preparedCert))),
+                        roundIdentifier, earlierPreparedCert))),
             proposal);
     proposerMessageFactory.createSignedProposalPayload(roundIdentifier, proposedBlock);