Spilt Ibft MessageValidator into components

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

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

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

@@ -17,6 +17,7 @@
 import tech.pegasys.pantheon.consensus.ibft.blockcreation.IbftBlockCreator;
 import tech.pegasys.pantheon.consensus.ibft.blockcreation.IbftBlockCreatorFactory;
 import tech.pegasys.pantheon.consensus.ibft.validation.MessageValidator;
+import tech.pegasys.pantheon.consensus.ibft.validation.SignedDataValidator;
 import tech.pegasys.pantheon.ethereum.BlockValidator;
 import tech.pegasys.pantheon.ethereum.ProtocolContext;
 import tech.pegasys.pantheon.ethereum.chain.MinedBlockObserver;
@@ -56,12 +57,12 @@ public IbftRound createNewRound(final BlockHeader parentHeader, final int round)
             roundIdentifier,
             finalState.getQuorum(),
             new MessageValidator(
-                finalState.getValidators(),
-                finalState.getProposerForRound(roundIdentifier),
-                roundIdentifier,
-                blockValidator,
-                protocolContext,
-                parentHeader));
+                new SignedDataValidator(
+                    finalState.getValidators(),
+                    finalState.getProposerForRound(roundIdentifier),
+                    roundIdentifier,
+                    blockValidator,
+                    protocolContext)));
 
     return createNewRoundWithState(parentHeader, roundState);
   }

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

@@ -66,10 +66,10 @@ public ConsensusRoundIdentifier getRoundIdentifier() {
   public boolean setProposedBlock(final Proposal msg) {
 
     if (!proposalMessage.isPresent()) {
-      if (validator.addSignedProposalPayload(msg.getSignedPayload())) {
+      if (validator.addSignedProposalPayload(msg)) {
         proposalMessage = Optional.of(msg);
-        preparePayloads.removeIf(p -> !validator.validatePrepareMessage(p.getSignedPayload()));
-        commitPayloads.removeIf(p -> !validator.validateCommmitMessage(p.getSignedPayload()));
+        preparePayloads.removeIf(p -> !validator.validatePrepareMessage(p));
+        commitPayloads.removeIf(p -> !validator.validateCommitMessage(p));
         updateState();
         return true;
       }
@@ -79,15 +79,15 @@ public boolean setProposedBlock(final Proposal msg) {
   }
 
   public void addPrepareMessage(final Prepare msg) {
-    if (!proposalMessage.isPresent() || validator.validatePrepareMessage(msg.getSignedPayload())) {
+    if (!proposalMessage.isPresent() || validator.validatePrepareMessage(msg)) {
       preparePayloads.add(msg);
       LOG.debug("Round state added prepare message prepare={}", msg);
     }
     updateState();
   }
 
   public void addCommitMessage(final Commit msg) {
-    if (!proposalMessage.isPresent() || validator.validateCommmitMessage(msg.getSignedPayload())) {
+    if (!proposalMessage.isPresent() || validator.validateCommitMessage(msg)) {
       commitPayloads.add(msg);
       LOG.debug("Round state added commit message commit={}", msg);
     }

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

@@ -12,205 +12,27 @@
  */
 package tech.pegasys.pantheon.consensus.ibft.validation;
 
-import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
-import tech.pegasys.pantheon.consensus.ibft.IbftContext;
-import tech.pegasys.pantheon.consensus.ibft.IbftExtraData;
-import tech.pegasys.pantheon.consensus.ibft.payload.CommitPayload;
-import tech.pegasys.pantheon.consensus.ibft.payload.Payload;
-import tech.pegasys.pantheon.consensus.ibft.payload.PreparePayload;
-import tech.pegasys.pantheon.consensus.ibft.payload.ProposalPayload;
-import tech.pegasys.pantheon.consensus.ibft.payload.SignedData;
-import tech.pegasys.pantheon.ethereum.BlockValidator;
-import tech.pegasys.pantheon.ethereum.BlockValidator.BlockProcessingOutputs;
-import tech.pegasys.pantheon.ethereum.ProtocolContext;
-import tech.pegasys.pantheon.ethereum.core.Address;
-import tech.pegasys.pantheon.ethereum.core.Block;
-import tech.pegasys.pantheon.ethereum.core.BlockHeader;
-import tech.pegasys.pantheon.ethereum.core.Hash;
-import tech.pegasys.pantheon.ethereum.core.Util;
-import tech.pegasys.pantheon.ethereum.mainnet.HeaderValidationMode;
-
-import java.util.Collection;
-import java.util.Optional;
-
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
+import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Commit;
+import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Prepare;
+import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Proposal;
 
 public class MessageValidator {
 
-  private static final Logger LOG = LogManager.getLogger();
-
-  private final Collection<Address> validators;
-  private final Address expectedProposer;
-  private final ConsensusRoundIdentifier roundIdentifier;
-  private final BlockValidator<IbftContext> blockValidator;
-  private final ProtocolContext<IbftContext> protocolContext;
-  private final BlockHeader parentHeader;
-
-  private Optional<SignedData<ProposalPayload>> proposal = Optional.empty();
-
-  public MessageValidator(
-      final Collection<Address> validators,
-      final Address expectedProposer,
-      final ConsensusRoundIdentifier roundIdentifier,
-      final BlockValidator<IbftContext> blockValidator,
-      final ProtocolContext<IbftContext> protocolContext,
-      final BlockHeader parentHeader) {
-    this.validators = validators;
-    this.expectedProposer = expectedProposer;
-    this.roundIdentifier = roundIdentifier;
-    this.blockValidator = blockValidator;
-    this.protocolContext = protocolContext;
-    this.parentHeader = parentHeader;
-  }
-
-  public boolean addSignedProposalPayload(final SignedData<ProposalPayload> msg) {
-
-    if (proposal.isPresent()) {
-      return handleSubsequentProposal(proposal.get(), msg);
-    }
-
-    if (!validateSignedProposalPayload(msg)) {
-      return false;
-    }
-
-    if (!validateBlockMatchesProposalRound(msg.getPayload())) {
-      return false;
-    }
-
-    proposal = Optional.of(msg);
-    return true;
-  }
-
-  private boolean validateSignedProposalPayload(final SignedData<ProposalPayload> msg) {
-
-    if (!msg.getPayload().getRoundIdentifier().equals(roundIdentifier)) {
-      LOG.info("Invalid Proposal message, does not match current round.");
-      return false;
-    }
-
-    if (!msg.getAuthor().equals(expectedProposer)) {
-      LOG.info(
-          "Invalid Proposal message, was not created by the proposer expected for the "
-              + "associated round.");
-      return false;
-    }
-
-    final Block proposedBlock = msg.getPayload().getBlock();
-
-    final Optional<BlockProcessingOutputs> validationResult =
-        blockValidator.validateAndProcessBlock(
-            protocolContext, proposedBlock, HeaderValidationMode.LIGHT, HeaderValidationMode.FULL);
-
-    if (!validationResult.isPresent()) {
-      LOG.info("Invalid Proposal message, block did not pass validation.");
-      return false;
-    }
-
-    return true;
-  }
-
-  private boolean handleSubsequentProposal(
-      final SignedData<ProposalPayload> existingMsg, final SignedData<ProposalPayload> newMsg) {
-    if (!existingMsg.getAuthor().equals(newMsg.getAuthor())) {
-      LOG.debug("Received subsequent invalid Proposal message; sender differs from original.");
-      return false;
-    }
-
-    final ProposalPayload existingData = existingMsg.getPayload();
-    final ProposalPayload newData = newMsg.getPayload();
-
-    if (!proposalMessagesAreIdentical(existingData, newData)) {
-      LOG.debug("Received subsequent invalid Proposal message; content differs from original.");
-      return false;
-    }
-
-    return true;
-  }
-
-  public boolean validatePrepareMessage(final SignedData<PreparePayload> msg) {
-    final String msgType = "Prepare";
-
-    if (!isMessageForCurrentRoundFromValidatorAndProposalAvailable(msg, msgType)) {
-      return false;
-    }
-
-    if (msg.getAuthor().equals(expectedProposer)) {
-      LOG.info("Illegal Prepare message; was sent by the round's proposer.");
-      return false;
-    }
-
-    return validateDigestMatchesProposal(msg.getPayload().getDigest(), msgType);
-  }
-
-  public boolean validateCommmitMessage(final SignedData<CommitPayload> msg) {
-    final String msgType = "Commit";
-
-    if (!isMessageForCurrentRoundFromValidatorAndProposalAvailable(msg, msgType)) {
-      return false;
-    }
-
-    final Block proposedBlock = proposal.get().getPayload().getBlock();
-    final Address commitSealCreator =
-        Util.signatureToAddress(msg.getPayload().getCommitSeal(), proposedBlock.getHash());
-
-    if (!commitSealCreator.equals(msg.getAuthor())) {
-      LOG.info("Invalid Commit message. Seal was not created by the message transmitter.");
-      return false;
-    }
-
-    return validateDigestMatchesProposal(msg.getPayload().getDigest(), msgType);
-  }
-
-  private boolean isMessageForCurrentRoundFromValidatorAndProposalAvailable(
-      final SignedData<? extends Payload> msg, final String msgType) {
-
-    if (!msg.getPayload().getRoundIdentifier().equals(roundIdentifier)) {
-      LOG.info("Invalid {} message, does not match current round.", msgType);
-      return false;
-    }
-
-    if (!validators.contains(msg.getAuthor())) {
-      LOG.info(
-          "Invalid {} message, was not transmitted by a validator for the " + "associated round.",
-          msgType);
-      return false;
-    }
+  private final SignedDataValidator signedDataValidator;
 
-    if (!proposal.isPresent()) {
-      LOG.info(
-          "Unable to validate {} message. No Proposal exists against which to validate "
-              + "block digest.",
-          msgType);
-      return false;
-    }
-    return true;
+  public MessageValidator(final SignedDataValidator signedDataValidator) {
+    this.signedDataValidator = signedDataValidator;
   }
 
-  private boolean validateDigestMatchesProposal(final Hash digest, final String msgType) {
-    final Block proposedBlock = proposal.get().getPayload().getBlock();
-    if (!digest.equals(proposedBlock.getHash())) {
-      LOG.info(
-          "Illegal {} message, digest does not match the block in the Prepare Message.", msgType);
-      return false;
-    }
-    return true;
+  public boolean addSignedProposalPayload(final Proposal msg) {
+    return signedDataValidator.addSignedProposalPayload(msg.getSignedPayload());
   }
 
-  private boolean proposalMessagesAreIdentical(
-      final ProposalPayload right, final ProposalPayload left) {
-    return right.getBlock().getHash().equals(left.getBlock().getHash())
-        && right.getRoundIdentifier().equals(left.getRoundIdentifier());
+  public boolean validatePrepareMessage(final Prepare msg) {
+    return signedDataValidator.validatePrepareMessage(msg.getSignedPayload());
   }
 
-  private boolean validateBlockMatchesProposalRound(final ProposalPayload payload) {
-    final ConsensusRoundIdentifier msgRound = payload.getRoundIdentifier();
-    final IbftExtraData extraData =
-        IbftExtraData.decode(payload.getBlock().getHeader().getExtraData());
-    if (extraData.getRound() != msgRound.getRoundNumber()) {
-      LOG.info("Invalid Proposal message, round number in block does not match that in message.");
-      return false;
-    }
-    return true;
+  public boolean validateCommitMessage(final Commit msg) {
+    return signedDataValidator.validateCommmitMessage(msg.getSignedPayload());
   }
 }

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

@@ -41,26 +41,29 @@ public MessageValidatorFactory(
     this.protocolContext = protocolContext;
   }
 
-  public MessageValidator createMessageValidator(
-      final ConsensusRoundIdentifier roundIdentifier, final BlockHeader parentHeader) {
+  private SignedDataValidator createSignedDataValidator(
+      final ConsensusRoundIdentifier roundIdentifier) {
     final BlockValidator<IbftContext> blockValidator =
         protocolSchedule.getByBlockNumber(roundIdentifier.getSequenceNumber()).getBlockValidator();
 
-    return new MessageValidator(
+    return new SignedDataValidator(
         protocolContext.getConsensusState().getVoteTally().getValidators(),
         proposerSelector.selectProposerForRound(roundIdentifier),
         roundIdentifier,
         blockValidator,
-        protocolContext,
-        parentHeader);
+        protocolContext);
+  }
+
+  public MessageValidator createMessageValidator(final ConsensusRoundIdentifier roundIdentifier) {
+    return new MessageValidator(createSignedDataValidator(roundIdentifier));
   }
 
   public RoundChangeMessageValidator createRoundChangeMessageValidator(
       final BlockHeader parentHeader) {
     final Collection<Address> validators =
         protocolContext.getConsensusState().getVoteTally().getValidators();
     return new RoundChangeMessageValidator(
-        roundIdentifier -> createMessageValidator(roundIdentifier, parentHeader),
+        this::createSignedDataValidator,
         validators,
         prepareMessageCountForQuorum(
             IbftHelpers.calculateRequiredValidatorQuorum(validators.size())),
@@ -73,7 +76,7 @@ public NewRoundMessageValidator createNewRoundValidator(final BlockHeader parent
     return new NewRoundMessageValidator(
         validators,
         proposerSelector,
-        roundIdentifier -> createMessageValidator(roundIdentifier, parentHeader),
+        this::createSignedDataValidator,
         IbftHelpers.calculateRequiredValidatorQuorum(validators.size()),
         parentHeader.getNumber() + 1);
   }

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

@@ -80,7 +80,7 @@ public boolean validateNewRoundMessage(final SignedData<NewRoundPayload> msg) {
     }
 
     final SignedData<ProposalPayload> proposalPayload = payload.getProposalPayload();
-    final MessageValidator proposalValidator =
+    final SignedDataValidator proposalValidator =
         messageValidatorFactory.createAt(rootRoundIdentifier);
     if (!proposalValidator.addSignedProposalPayload(proposalPayload)) {
       LOG.info("Invalid NewRound message, embedded proposal failed validation");

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

@@ -81,15 +81,15 @@ private boolean validatePrepareCertificate(
       return false;
     }
 
-    final MessageValidator messageValidator =
+    final SignedDataValidator signedDataValidator =
         messageValidatorFactory.createAt(proposalRoundIdentifier);
-    return validateConsistencyOfPrepareCertificateMessages(certificate, messageValidator);
+    return validateConsistencyOfPrepareCertificateMessages(certificate, signedDataValidator);
   }
 
   private boolean validateConsistencyOfPrepareCertificateMessages(
-      final PreparedCertificate certificate, final MessageValidator messageValidator) {
+      final PreparedCertificate certificate, final SignedDataValidator signedDataValidator) {
 
-    if (!messageValidator.addSignedProposalPayload(certificate.getProposalPayload())) {
+    if (!signedDataValidator.addSignedProposalPayload(certificate.getProposalPayload())) {
       LOG.info("Invalid RoundChange message, embedded Proposal message failed validation.");
       return false;
     }
@@ -102,7 +102,7 @@ private boolean validateConsistencyOfPrepareCertificateMessages(
     }
 
     for (final SignedData<PreparePayload> prepareMsg : certificate.getPreparePayloads()) {
-      if (!messageValidator.validatePrepareMessage(prepareMsg)) {
+      if (!signedDataValidator.validatePrepareMessage(prepareMsg)) {
         LOG.info("Invalid RoundChange message, embedded Prepare message failed validation.");
         return false;
       }
@@ -130,6 +130,6 @@ private boolean validatePreparedCertificateRound(
 
   @FunctionalInterface
   public interface MessageValidatorForHeightFactory {
-    MessageValidator createAt(final ConsensusRoundIdentifier roundIdentifier);
+    SignedDataValidator createAt(final ConsensusRoundIdentifier roundIdentifier);
   }
 }