Wire qbft message validation into business logic

consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/TestContext.java

@@ -14,6 +14,7 @@
  */
 package org.hyperledger.besu.consensus.qbft.support;
 
+import org.hyperledger.besu.consensus.common.bft.BftBlockHeaderFunctions;
 import org.hyperledger.besu.consensus.common.bft.BftExecutors;
 import org.hyperledger.besu.consensus.common.bft.ConsensusRoundIdentifier;
 import org.hyperledger.besu.consensus.common.bft.EventMultiplexer;
@@ -25,6 +26,7 @@
 import org.hyperledger.besu.ethereum.core.Address;
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.BlockHeaderBuilder;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -86,13 +88,28 @@ public MessageFactory getLocalNodeMessageFactory() {
     return messageFactory;
   }
 
+  public Block createBlockForProposalFromChainHead(final int round, final long timestamp) {
+    return createBlockForProposalFromChainHead(round, timestamp, finalState.getLocalAddress());
+  }
+
   public Block createBlockForProposal(
-      final BlockHeader parent, final int round, final long timestamp) {
-    return finalState.getBlockCreatorFactory().create(parent, round).createBlock(timestamp);
+      final BlockHeader parent, final int round, final long timestamp, final Address proposer) {
+    final Block block =
+        finalState.getBlockCreatorFactory().create(parent, round).createBlock(timestamp);
+
+    final BlockHeaderBuilder headerBuilder = BlockHeaderBuilder.fromHeader(block.getHeader());
+    headerBuilder
+        .coinbase(proposer)
+        .blockHeaderFunctions(BftBlockHeaderFunctions.forCommittedSeal());
+    final BlockHeader newHeader = headerBuilder.buildBlockHeader();
+
+    return new Block(newHeader, block.getBody());
   }
 
-  public Block createBlockForProposalFromChainHead(final int round, final long timestamp) {
-    return createBlockForProposal(blockchain.getChainHeadHeader(), round, timestamp);
+  public Block createBlockForProposalFromChainHead(
+      final int round, final long timestamp, final Address proposer) {
+    // this implies that EVERY block will have this node as the proposer :/
+    return createBlockForProposal(blockchain.getChainHeadHeader(), round, timestamp, proposer);
   }
 
   public RoundSpecificPeers roundSpecificPeers(final ConsensusRoundIdentifier roundId) {

consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/test/FutureHeightTest.java

@@ -65,7 +65,8 @@ public void messagesForFutureHeightAreBufferedUntilChainHeightCatchesUp() {
         BftHelpers.createSealedBlock(currentHeightBlock, peers.sign(currentHeightBlock.getHash()));
 
     final Block futureHeightBlock =
-        context.createBlockForProposal(signedCurrentHeightBlock.getHeader(), 0, 60);
+        context.createBlockForProposal(
+            signedCurrentHeightBlock.getHeader(), 0, 60, peers.getProposer().getNodeAddress());
 
     peers.getProposer().injectProposal(futureHeightRoundId, futureHeightBlock);
     peers.verifyNoMessagesReceived();
@@ -107,7 +108,8 @@ public void messagesForFutureHeightAreBufferedUntilChainHeightCatchesUp() {
 
   @Test
   public void messagesFromPreviousHeightAreDiscarded() {
-    final Block currentHeightBlock = context.createBlockForProposalFromChainHead(0, 30);
+    final Block currentHeightBlock =
+        context.createBlockForProposalFromChainHead(0, 30, peers.getProposer().getNodeAddress());
     final Block signedCurrentHeightBlock =
         BftHelpers.createSealedBlock(currentHeightBlock, peers.sign(currentHeightBlock.getHash()));
 
@@ -137,7 +139,8 @@ public void messagesFromPreviousHeightAreDiscarded() {
 
   @Test
   public void multipleNewChainHeadEventsDoesNotRestartCurrentHeightManager() {
-    final Block currentHeightBlock = context.createBlockForProposalFromChainHead(0, 30);
+    final Block currentHeightBlock =
+        context.createBlockForProposalFromChainHead(0, 30, peers.getProposer().getNodeAddress());
 
     peers.getProposer().injectProposal(roundId, currentHeightBlock);
     peers.getProposer().injectPrepare(roundId, currentHeightBlock.getHash());
@@ -166,12 +169,14 @@ public void correctMessagesAreExtractedFromFutureHeightBuffer() {
         BftHelpers.createSealedBlock(currentHeightBlock, peers.sign(currentHeightBlock.getHash()));
 
     final Block nextHeightBlock =
-        context.createBlockForProposal(signedCurrentHeightBlock.getHeader(), 0, 60);
+        context.createBlockForProposal(
+            signedCurrentHeightBlock.getHeader(), 0, 60, peers.getProposer().getNodeAddress());
     final Block signedNextHeightBlock =
         BftHelpers.createSealedBlock(nextHeightBlock, peers.sign(nextHeightBlock.getHash()));
 
     final Block futureHeightBlock =
-        context.createBlockForProposal(signedNextHeightBlock.getHeader(), 0, 90);
+        context.createBlockForProposal(
+            signedNextHeightBlock.getHeader(), 0, 90, peers.getNonProposing(0).getNodeAddress());
 
     final ConsensusRoundIdentifier nextHeightRoundId = new ConsensusRoundIdentifier(2, 0);
     final ConsensusRoundIdentifier futureHeightRoundId = new ConsensusRoundIdentifier(3, 0);
@@ -197,7 +202,8 @@ public void correctMessagesAreExtractedFromFutureHeightBuffer() {
     // messages have not been used.
     peers.verifyMessagesReceived(expectedPrepareMessage);
 
-    peers.getProposer().injectProposal(futureHeightRoundId, futureHeightBlock);
+    // Future height proposal needs to come from the NEXT nonProposer.
+    peers.getNonProposing(0).injectProposal(futureHeightRoundId, futureHeightBlock);
 
     // Change to the FutureRound, and confirm prepare and commit msgs are sent
     context.getBlockchain().appendBlock(signedNextHeightBlock, emptyList());

consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/test/FutureRoundTest.java

@@ -63,7 +63,8 @@ public class FutureRoundTest {
   @Test
   public void messagesForFutureRoundAreNotActionedUntilRoundIsActive() {
     final Block futureBlock =
-        context.createBlockForProposalFromChainHead(futureRoundId.getRoundNumber(), 60);
+        context.createBlockForProposalFromChainHead(
+            futureRoundId.getRoundNumber(), 60, peers.getProposer().getNodeAddress());
     final int quorum = BftHelpers.calculateRequiredValidatorQuorum(NETWORK_SIZE);
     final ConsensusRoundIdentifier subsequentRoundId = new ConsensusRoundIdentifier(1, 6);
     final RoundSpecificPeers subsequentRoles = context.roundSpecificPeers(subsequentRoundId);
@@ -117,9 +118,11 @@ public void messagesForFutureRoundAreNotActionedUntilRoundIsActive() {
   @Test
   public void priorRoundsCannotBeCompletedAfterReceptionOfNewRound() {
     final Block initialBlock =
-        context.createBlockForProposalFromChainHead(roundId.getRoundNumber(), 30);
+        context.createBlockForProposalFromChainHead(
+            roundId.getRoundNumber(), 30, peers.getProposer().getNodeAddress());
     final Block futureBlock =
-        context.createBlockForProposalFromChainHead(futureRoundId.getRoundNumber(), 60);
+        context.createBlockForProposalFromChainHead(
+            futureRoundId.getRoundNumber(), 60, peers.getProposer().getNodeAddress());
 
     peers.getProposer().injectProposal(roundId, initialBlock);
 

consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/test/GossipTest.java

@@ -39,7 +39,6 @@
 import java.util.Optional;
 
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class GossipTest {
@@ -66,7 +65,9 @@ public class GossipTest {
 
   @Before
   public void setup() {
-    block = context.createBlockForProposalFromChainHead(roundId.getRoundNumber(), 30);
+    block =
+        context.createBlockForProposalFromChainHead(
+            roundId.getRoundNumber(), 30, peers.getProposer().getNodeAddress());
     sender = peers.getProposer();
     msgFactory = sender.getMessageFactory();
   }
@@ -128,7 +129,6 @@ public void messageWithUnknownValidatorIsNotGossiped() {
     peers.verifyNoMessagesReceived();
   }
 
-  @Ignore("Requires validation")
   @Test
   public void messageIsNotGossipedToSenderOrCreator() {
     final ValidatorPeer msgCreator = peers.getFirstNonProposer();

consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/test/LocalNodeNotProposerTest.java

@@ -45,7 +45,8 @@ public class LocalNodeNotProposerTest {
 
   private final MessageFactory localNodeMessageFactory = context.getLocalNodeMessageFactory();
 
-  private final Block blockToPropose = context.createBlockForProposalFromChainHead(0, 15);
+  private final Block blockToPropose =
+      context.createBlockForProposalFromChainHead(0, 15, peers.getProposer().getNodeAddress());
 
   private Prepare expectedTxPrepare;
   private Commit expectedTxCommit;

consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/test/ReceivedFutureProposalTest.java

@@ -21,6 +21,7 @@
 import org.hyperledger.besu.consensus.qbft.messagewrappers.Commit;
 import org.hyperledger.besu.consensus.qbft.messagewrappers.Prepare;
 import org.hyperledger.besu.consensus.qbft.payload.MessageFactory;
+import org.hyperledger.besu.consensus.qbft.payload.PreparePayload;
 import org.hyperledger.besu.consensus.qbft.payload.RoundChangePayload;
 import org.hyperledger.besu.consensus.qbft.statemachine.PreparedCertificate;
 import org.hyperledger.besu.consensus.qbft.support.IntegrationTestHelpers;
@@ -32,8 +33,8 @@
 
 import java.util.Collections;
 import java.util.List;
+import java.util.stream.Collectors;
 
-import org.junit.Ignore;
 import org.junit.Test;
 
 /**
@@ -57,15 +58,15 @@ public class ReceivedFutureProposalTest {
 
   @Test
   public void proposalWithEmptyPrepareCertificatesOfferNewBlock() {
-    final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
-    final Block blockToPropose =
-        context.createBlockForProposalFromChainHead(nextRoundId.getRoundNumber(), 15);
     final ConsensusRoundIdentifier targetRound = new ConsensusRoundIdentifier(1, 1);
 
     final List<SignedData<RoundChangePayload>> roundChanges =
         peers.createSignedRoundChangePayload(targetRound);
 
-    final ValidatorPeer nextProposer = context.roundSpecificPeers(nextRoundId).getProposer();
+    final ValidatorPeer nextProposer = context.roundSpecificPeers(targetRound).getProposer();
+    final Block blockToPropose =
+        context.createBlockForProposalFromChainHead(
+            targetRound.getRoundNumber(), 15, nextProposer.getNodeAddress());
 
     nextProposer.injectProposalForFutureRound(
         targetRound, roundChanges, Collections.emptyList(), blockToPropose);
@@ -76,12 +77,12 @@ public void proposalWithEmptyPrepareCertificatesOfferNewBlock() {
     peers.verifyMessagesReceived(expectedPrepare);
   }
 
-  @Ignore("Requires validation")
   @Test
   public void proposalFromIllegalSenderIsDiscardedAndNoPrepareForNewRoundIsSent() {
     final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
     final Block blockToPropose =
-        context.createBlockForProposalFromChainHead(nextRoundId.getRoundNumber(), 15);
+        context.createBlockForProposalFromChainHead(
+            nextRoundId.getRoundNumber(), 15, peers.getProposer().getNodeAddress());
 
     final List<SignedData<RoundChangePayload>> roundChanges =
         peers.createSignedRoundChangePayload(nextRoundId);
@@ -97,8 +98,10 @@ public void proposalFromIllegalSenderIsDiscardedAndNoPrepareForNewRoundIsSent()
 
   @Test
   public void proposalWithPrepareCertificateResultsInNewRoundStartingWithExpectedBlock() {
-    final Block initialBlock = context.createBlockForProposalFromChainHead(0, 15);
-    final Block reproposedBlock = context.createBlockForProposalFromChainHead(1, 15);
+    final Block initialBlock =
+        context.createBlockForProposalFromChainHead(0, 15, peers.getProposer().getNodeAddress());
+    final Block reproposedBlock =
+        context.createBlockForProposalFromChainHead(1, 15, peers.getProposer().getNodeAddress());
     final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
 
     final PreparedCertificate preparedRoundArtifacts =
@@ -107,15 +110,78 @@ public void proposalWithPrepareCertificateResultsInNewRoundStartingWithExpectedB
     final List<SignedData<RoundChangePayload>> roundChanges =
         peers.createSignedRoundChangePayload(nextRoundId, preparedRoundArtifacts);
 
+    final List<SignedData<PreparePayload>> prepares =
+        peers.createSignedPreparePayloadOfAllPeers(roundId, initialBlock.getHash());
+
     final ValidatorPeer nextProposer = context.roundSpecificPeers(nextRoundId).getProposer();
 
-    nextProposer.injectProposalForFutureRound(
-        nextRoundId, roundChanges, Collections.emptyList(), reproposedBlock);
+    nextProposer.injectProposalForFutureRound(nextRoundId, roundChanges, prepares, reproposedBlock);
 
     peers.verifyMessagesReceived(
         localNodeMessageFactory.createPrepare(nextRoundId, reproposedBlock.getHash()));
   }
 
+  @Test
+  public void futureProposalWithInsufficientPreparesDoesNotTriggerNextRound() {
+    final Block initialBlock =
+        context.createBlockForProposalFromChainHead(0, 15, peers.getProposer().getNodeAddress());
+    final Block reproposedBlock =
+        context.createBlockForProposalFromChainHead(1, 15, peers.getProposer().getNodeAddress());
+    final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
+
+    final PreparedCertificate preparedRoundArtifacts =
+        createValidPreparedCertificate(context, roundId, initialBlock);
+
+    final List<SignedData<RoundChangePayload>> roundChanges =
+        peers.createSignedRoundChangePayload(nextRoundId, preparedRoundArtifacts);
+
+    final List<SignedData<PreparePayload>> prepares =
+        peers.createSignedPreparePayloadOfAllPeers(roundId, initialBlock.getHash());
+
+    final ValidatorPeer nextProposer = context.roundSpecificPeers(nextRoundId).getProposer();
+
+    nextProposer.injectProposalForFutureRound(
+        nextRoundId, roundChanges, prepares.subList(0, 2), reproposedBlock);
+
+    peers.verifyNoMessagesReceived();
+  }
+
+  @Test
+  public void futureProposalWithInvalidPrepareDoesNotTriggerNextRound() {
+    final Block initialBlock =
+        context.createBlockForProposalFromChainHead(0, 15, peers.getProposer().getNodeAddress());
+    final Block reproposedBlock = context.createBlockForProposalFromChainHead(1, 15);
+    final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
+
+    final PreparedCertificate preparedRoundArtifacts =
+        createValidPreparedCertificate(context, roundId, initialBlock);
+
+    final List<SignedData<RoundChangePayload>> roundChanges =
+        peers.createSignedRoundChangePayload(nextRoundId, preparedRoundArtifacts);
+
+    List<SignedData<PreparePayload>> prepares =
+        peers.createSignedPreparePayloadOfAllPeers(roundId, initialBlock.getHash());
+    prepares =
+        prepares.stream()
+            .filter(p -> !p.getAuthor().equals(peers.getFirstNonProposer().getNodeAddress()))
+            .collect(Collectors.toList());
+
+    final SignedData<PreparePayload> invalidPrepare =
+        peers
+            .getFirstNonProposer()
+            .getMessageFactory()
+            .createPrepare(nextRoundId, initialBlock.getHash())
+            .getSignedPayload();
+
+    prepares.add(invalidPrepare);
+
+    final ValidatorPeer nextProposer = context.roundSpecificPeers(nextRoundId).getProposer();
+
+    nextProposer.injectProposalForFutureRound(nextRoundId, roundChanges, prepares, reproposedBlock);
+
+    peers.verifyNoMessagesReceived();
+  }
+
   @Test
   public void proposalMessageForPriorRoundIsNotActioned() {
     // first move to a future round, then inject a proposal for a prior round, local node
@@ -141,8 +207,10 @@ public void proposalMessageForPriorRoundIsNotActioned() {
 
   @Test
   public void receiveRoundStateIsNotLostIfASecondProposalMessageIsReceivedForCurrentRound() {
-    final Block initialBlock = context.createBlockForProposalFromChainHead(0, 15);
-    final Block reproposedBlock = context.createBlockForProposalFromChainHead(1, 15);
+    final Block initialBlock =
+        context.createBlockForProposalFromChainHead(0, 15, peers.getProposer().getNodeAddress());
+    final Block reproposedBlock =
+        context.createBlockForProposalFromChainHead(1, 15, peers.getProposer().getNodeAddress());
     final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
 
     final PreparedCertificate preparedRoundArtifacts =