Refactor RoundChangeManager creation (PegaSysEng#578)

Constructor of the RoundChangeManager now accepts a
RoundChangeMessageValidator rather than creating it from inputs.

This removes complexity from this class, pushes creation back to the
validator factory, and simplifies the IbftBlockHeightManagerFactory.

consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextFactory.java

@@ -271,8 +271,7 @@ private static ControllerAndState createControllerAndFinalState(
                 finalState,
                 new IbftRoundFactory(
                     finalState, protocolContext, protocolSchedule, minedBlockObservers),
-                messageValidatorFactory,
-                protocolContext),
+                messageValidatorFactory),
             new HashMap<>(),
             gossiper);
     //////////////////////////// END IBFT PantheonController ////////////////////////////

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

@@ -102,7 +102,7 @@ public IbftBlockHeightManager(
         (roundIdentifier) ->
             new RoundState(
                 roundIdentifier,
-                finalState.getQuorumSize(),
+                finalState.getQuorum(),
                 messageValidatorFactory.createMessageValidator(roundIdentifier, parentHeader));
   }
 

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

@@ -12,39 +12,32 @@
  */
 package tech.pegasys.pantheon.consensus.ibft.statemachine;
 
-import tech.pegasys.pantheon.consensus.ibft.IbftContext;
+import tech.pegasys.pantheon.consensus.ibft.IbftHelpers;
 import tech.pegasys.pantheon.consensus.ibft.validation.MessageValidatorFactory;
-import tech.pegasys.pantheon.ethereum.ProtocolContext;
 import tech.pegasys.pantheon.ethereum.core.BlockHeader;
 
 public class IbftBlockHeightManagerFactory {
 
   private final IbftRoundFactory roundFactory;
   private final IbftFinalState finalState;
-  private final ProtocolContext<IbftContext> protocolContext;
   private final MessageValidatorFactory messageValidatorFactory;
 
   public IbftBlockHeightManagerFactory(
       final IbftFinalState finalState,
       final IbftRoundFactory roundFactory,
-      final MessageValidatorFactory messageValidatorFactory,
-      final ProtocolContext<IbftContext> protocolContext) {
+      final MessageValidatorFactory messageValidatorFactory) {
     this.roundFactory = roundFactory;
     this.finalState = finalState;
-    this.protocolContext = protocolContext;
     this.messageValidatorFactory = messageValidatorFactory;
   }
 
   public IbftBlockHeightManager create(final BlockHeader parentHeader) {
-    long nextChainHeight = parentHeader.getNumber() + 1;
     return new IbftBlockHeightManager(
         parentHeader,
         finalState,
         new RoundChangeManager(
-            nextChainHeight,
-            finalState.getValidators(),
-            (roundIdentifier) ->
-                messageValidatorFactory.createMessageValidator(roundIdentifier, parentHeader)),
+            IbftHelpers.calculateRequiredValidatorQuorum(finalState.getValidators().size()),
+            messageValidatorFactory.createRoundChangeMessageValidator(parentHeader)),
         roundFactory,
         finalState.getClock(),
         messageValidatorFactory);

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

@@ -71,7 +71,7 @@ public IbftFinalState(
     this.messageTransmitter = new IbftMessageTransmitter(messageFactory, peers);
   }
 
-  public int getQuorumSize() {
+  public int getQuorum() {
     return calculateRequiredValidatorQuorum(validatorProvider.getValidators().size());
   }
 

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

@@ -50,7 +50,7 @@ public IbftRound createNewRound(final BlockHeader parentHeader, final int round)
     final RoundState roundState =
         new RoundState(
             roundIdentifier,
-            finalState.getQuorumSize(),
+            finalState.getQuorum(),
             new MessageValidator(
                 finalState.getValidators(),
                 finalState.getProposerForRound(roundIdentifier),

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

@@ -12,18 +12,13 @@
  */
 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.payload.RoundChangeCertificate;
 import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangePayload;
 import tech.pegasys.pantheon.consensus.ibft.payload.SignedData;
 import tech.pegasys.pantheon.consensus.ibft.validation.RoundChangeMessageValidator;
-import tech.pegasys.pantheon.consensus.ibft.validation.RoundChangeMessageValidator.MessageValidatorForHeightFactory;
 import tech.pegasys.pantheon.ethereum.core.Address;
 
-import java.util.Collection;
 import java.util.Map;
 import java.util.Optional;
 
@@ -44,16 +39,17 @@
 public class RoundChangeManager {
 
   public static class RoundChangeStatus {
-    private final int quorumSize;
+
+    private final long quorum;
 
     // Store only 1 round change per round per validator
     @VisibleForTesting
     final Map<Address, SignedData<RoundChangePayload>> receivedMessages = Maps.newLinkedHashMap();
 
     private boolean actioned = false;
 
-    public RoundChangeStatus(final int quorumSize) {
-      this.quorumSize = quorumSize;
+    public RoundChangeStatus(final long quorum) {
+      this.quorum = quorum;
     }
 
     public void addMessage(final SignedData<RoundChangePayload> msg) {
@@ -63,7 +59,7 @@ public void addMessage(final SignedData<RoundChangePayload> msg) {
     }
 
     public boolean roundChangeReady() {
-      return receivedMessages.size() >= quorumSize && !actioned;
+      return receivedMessages.size() >= quorum && !actioned;
     }
 
     public RoundChangeCertificate createRoundChangeCertificate() {
@@ -81,20 +77,13 @@ public RoundChangeCertificate createRoundChangeCertificate() {
   @VisibleForTesting
   final Map<ConsensusRoundIdentifier, RoundChangeStatus> roundChangeCache = Maps.newHashMap();
 
-  private final int quorumSize;
+  private final long quorum;
   private final RoundChangeMessageValidator roundChangeMessageValidator;
 
   public RoundChangeManager(
-      final long sequenceNumber,
-      final Collection<Address> validators,
-      final MessageValidatorForHeightFactory messageValidityFactory) {
-    this.quorumSize = IbftHelpers.calculateRequiredValidatorQuorum(validators.size());
-    this.roundChangeMessageValidator =
-        new RoundChangeMessageValidator(
-            messageValidityFactory,
-            validators,
-            prepareMessageCountForQuorum(quorumSize),
-            sequenceNumber);
+      final long quorum, final RoundChangeMessageValidator roundChangeMessageValidator) {
+    this.quorum = quorum;
+    this.roundChangeMessageValidator = roundChangeMessageValidator;
   }
 
   /**
@@ -129,8 +118,7 @@ private RoundChangeStatus storeRoundChangeMessage(final SignedData<RoundChangePa
     final ConsensusRoundIdentifier msgTargetRound = msg.getPayload().getRoundIdentifier();
 
     final RoundChangeStatus roundChangeStatus =
-        roundChangeCache.computeIfAbsent(
-            msgTargetRound, ignored -> new RoundChangeStatus(quorumSize));
+        roundChangeCache.computeIfAbsent(msgTargetRound, ignored -> new RoundChangeStatus(quorum));
 
     roundChangeStatus.addMessage(msg);
 

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

@@ -36,7 +36,7 @@ public class RoundState {
 
   private final ConsensusRoundIdentifier roundIdentifier;
   private final MessageValidator validator;
-  private final long quorumSize;
+  private final long quorum;
 
   private Optional<SignedData<ProposalPayload>> proposalMessage = Optional.empty();
 
@@ -50,10 +50,10 @@ public class RoundState {
 
   public RoundState(
       final ConsensusRoundIdentifier roundIdentifier,
-      final int quorumSize,
+      final int quorum,
       final MessageValidator validator) {
     this.roundIdentifier = roundIdentifier;
-    this.quorumSize = quorumSize;
+    this.quorum = quorum;
     this.validator = validator;
   }
 
@@ -92,12 +92,12 @@ public void addCommitMessage(final SignedData<CommitPayload> msg) {
   }
 
   private void updateState() {
-    // NOTE: The quorumSize for Prepare messages is 1 less than the quorum size as the proposer
+    // NOTE: The quorum for Prepare messages is 1 less than the quorum size as the proposer
     // does not supply a prepare message
     prepared =
-        (preparePayloads.size() >= prepareMessageCountForQuorum(quorumSize))
+        (preparePayloads.size() >= prepareMessageCountForQuorum(quorum))
             && proposalMessage.isPresent();
-    committed = (commitPayloads.size() >= quorumSize) && proposalMessage.isPresent();
+    committed = (commitPayloads.size() >= quorum) && proposalMessage.isPresent();
   }
 
   public Optional<Block> getProposedBlock() {

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

@@ -41,19 +41,19 @@ public class NewRoundMessageValidator {
   private final Collection<Address> validators;
   private final ProposerSelector proposerSelector;
   private final MessageValidatorForHeightFactory messageValidatorFactory;
-  private final long quorumSize;
+  private final long quorum;
   private final long chainHeight;
 
   public NewRoundMessageValidator(
       final Collection<Address> validators,
       final ProposerSelector proposerSelector,
       final MessageValidatorForHeightFactory messageValidatorFactory,
-      final long quorumSize,
+      final long quorum,
       final long chainHeight) {
     this.validators = validators;
     this.proposerSelector = proposerSelector;
     this.messageValidatorFactory = messageValidatorFactory;
-    this.quorumSize = quorumSize;
+    this.quorum = quorum;
     this.chainHeight = chainHeight;
   }
 
@@ -101,7 +101,7 @@ private boolean validateRoundChangeMessagesAndEnsureTargetRoundMatchesRoot(
     final Collection<SignedData<RoundChangePayload>> roundChangeMsgs =
         roundChangeCert.getRoundChangePayloads();
 
-    if (roundChangeMsgs.size() < quorumSize) {
+    if (roundChangeMsgs.size() < quorum) {
       LOG.info(
           "Invalid NewRound message, RoundChange certificate has insufficient "
               + "RoundChange messages.");
@@ -124,7 +124,7 @@ private boolean validateRoundChangeMessagesAndEnsureTargetRoundMatchesRoot(
           new RoundChangeMessageValidator(
               messageValidatorFactory,
               validators,
-              prepareMessageCountForQuorum(quorumSize),
+              prepareMessageCountForQuorum(quorum),
               chainHeight);
 
       if (!roundChangeValidator.validateMessage(roundChangeMsg)) {

consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java

@@ -132,7 +132,7 @@ public void setup() {
     when(finalState.getTransmitter()).thenReturn(messageTransmitter);
     when(finalState.getBlockTimer()).thenReturn(blockTimer);
     when(finalState.getRoundTimer()).thenReturn(roundTimer);
-    when(finalState.getQuorumSize()).thenReturn(3);
+    when(finalState.getQuorum()).thenReturn(3);
     when(finalState.getMessageFactory()).thenReturn(messageFactory);
     when(blockCreator.createBlock(anyLong())).thenReturn(createdBlock);
     when(newRoundMessageValidator.validateNewRoundMessage(any())).thenReturn(true);
@@ -150,7 +150,7 @@ public void setup() {
               final int round = (int) invocation.getArgument(1);
               final ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(1, round);
               final RoundState createdRoundState =
-                  new RoundState(roundId, finalState.getQuorumSize(), messageValidator);
+                  new RoundState(roundId, finalState.getQuorum(), messageValidator);
               return new IbftRound(
                   createdRoundState,
                   blockCreator,

consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeManagerTest.java

@@ -19,6 +19,7 @@
 
 import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
 import tech.pegasys.pantheon.consensus.ibft.IbftContext;
+import tech.pegasys.pantheon.consensus.ibft.IbftHelpers;
 import tech.pegasys.pantheon.consensus.ibft.TestHelpers;
 import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory;
 import tech.pegasys.pantheon.consensus.ibft.payload.PreparePayload;
@@ -113,7 +114,14 @@ public void setup() {
                     protocolContext,
                     parentHeader));
 
-    manager = new RoundChangeManager(2, validators, messageValidatorFactory);
+    final RoundChangeMessageValidator roundChangeMessageValidator =
+        new RoundChangeMessageValidator(
+            messageValidatorFactory,
+            validators,
+            IbftHelpers.calculateRequiredValidatorQuorum(
+                IbftHelpers.calculateRequiredValidatorQuorum(validators.size())),
+            2);
+    manager = new RoundChangeManager(2, roundChangeMessageValidator);
   }
 
   private SignedData<RoundChangePayload> makeRoundChangeMessage(
@@ -248,7 +256,8 @@ public void roundChangeMessagesWithPreparedCertificateMustHaveSufficientPrepareM
     assertThat(manager.roundChangeCache.get(ri2)).isNull();
 
     roundChangeData =
-        makeRoundChangeMessageWithPreparedCert(proposerKey, ri2, Lists.newArrayList(validator1Key));
+        makeRoundChangeMessageWithPreparedCert(
+            proposerKey, ri2, Lists.newArrayList(validator1Key, validator2Key));
     assertThat(manager.appendRoundChangeMessage(roundChangeData)).isEmpty();
     assertThat(manager.roundChangeCache.get(ri2).receivedMessages.size()).isEqualTo(1);
   }

pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java

@@ -241,8 +241,7 @@ public static PantheonController<IbftContext> init(
                 finalState,
                 new IbftRoundFactory(
                     finalState, protocolContext, protocolSchedule, minedBlockObservers),
-                messageValidatorFactory,
-                protocolContext));
+                messageValidatorFactory));
 
     final IbftProcessor ibftProcessor = new IbftProcessor(ibftEventQueue, ibftController);
     final ExecutorService processorExecutor = Executors.newSingleThreadExecutor();