Ibft Integration Test - Future Rounds (#542)

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

@@ -12,17 +12,28 @@
  */
 package tech.pegasys.pantheon.consensus.ibft.support;
 
+import static java.util.Optional.empty;
+
 import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
 import tech.pegasys.pantheon.consensus.ibft.IbftBlockHashing;
 import tech.pegasys.pantheon.consensus.ibft.IbftExtraData;
 import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.CommitPayload;
 import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.MessageFactory;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.NewRoundPayload;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparedCertificate;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangeCertificate;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangePayload;
 import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData;
 import tech.pegasys.pantheon.crypto.SECP256K1;
 import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair;
 import tech.pegasys.pantheon.crypto.SECP256K1.Signature;
 import tech.pegasys.pantheon.ethereum.core.Block;
 
+import java.util.Collection;
+import java.util.List;
+import java.util.stream.Collectors;
+
 public class TestHelpers {
 
   public static SignedData<CommitPayload> createSignedCommentPayload(
@@ -39,4 +50,39 @@ public class TestHelpers {
 
     return messageFactory.createSignedCommitPayload(roundId, block.getHash(), commitSeal);
   }
+
+  public static PreparedCertificate createValidPreparedCertificate(
+      final TestContext context, final ConsensusRoundIdentifier preparedRound, final Block block) {
+    final RoundSpecificNodeRoles roles = context.getRoundSpecificRoles(preparedRound);
+
+    return new PreparedCertificate(
+        roles.getProposer().getMessageFactory().createSignedProposalPayload(preparedRound, block),
+        roles
+            .getNonProposingPeers()
+            .stream()
+            .map(
+                role ->
+                    role.getMessageFactory()
+                        .createSignedPreparePayload(preparedRound, block.getHash()))
+            .collect(Collectors.toList()));
+  }
+
+  public static SignedData<NewRoundPayload> injectEmptyNewRound(
+      final ConsensusRoundIdentifier targetRoundId,
+      final ValidatorPeer proposer,
+      final Collection<ValidatorPeer> peers,
+      final Block blockToPropose) {
+
+    final List<SignedData<RoundChangePayload>> roundChangePayloads =
+        peers
+            .stream()
+            .map(p -> p.getMessageFactory().createSignedRoundChangePayload(targetRoundId, empty()))
+            .collect(Collectors.toList());
+
+    final SignedData<ProposalPayload> proposal =
+        proposer.getMessageFactory().createSignedProposalPayload(targetRoundId, blockToPropose);
+
+    return proposer.injectNewRound(
+        targetRoundId, new RoundChangeCertificate(roundChangePayloads), proposal);
+  }
 }

consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/FutureRoundTest.java

@@ -0,0 +1,147 @@
+/*
+ * Copyright 2019 ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+package tech.pegasys.pantheon.consensus.ibft.tests;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static tech.pegasys.pantheon.consensus.ibft.support.MessageReceptionHelpers.assertPeersReceivedExactly;
+import static tech.pegasys.pantheon.consensus.ibft.support.MessageReceptionHelpers.assertPeersReceivedNoMessages;
+import static tech.pegasys.pantheon.consensus.ibft.support.TestHelpers.injectEmptyNewRound;
+
+import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
+import tech.pegasys.pantheon.consensus.ibft.IbftHelpers;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.CommitPayload;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.MessageFactory;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparePayload;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData;
+import tech.pegasys.pantheon.consensus.ibft.support.RoundSpecificNodeRoles;
+import tech.pegasys.pantheon.consensus.ibft.support.TestContext;
+import tech.pegasys.pantheon.consensus.ibft.support.TestContextFactory;
+import tech.pegasys.pantheon.consensus.ibft.support.ValidatorPeer;
+import tech.pegasys.pantheon.crypto.SECP256K1;
+import tech.pegasys.pantheon.ethereum.core.Block;
+
+import java.time.Clock;
+import java.time.Instant;
+import java.time.ZoneId;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class FutureRoundTest {
+
+  private final long blockTimeStamp = 100;
+  private final Clock fixedClock =
+      Clock.fixed(Instant.ofEpochSecond(blockTimeStamp), ZoneId.systemDefault());
+
+  private final int NETWORK_SIZE = 5;
+
+  // Local node is not the first proposer for the current round.
+  private final TestContext context =
+      TestContextFactory.createTestEnvironment(NETWORK_SIZE, 0, fixedClock);
+
+  private final ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(1, 0);
+  private final RoundSpecificNodeRoles roles = context.getRoundSpecificRoles(roundId);
+
+  private final ConsensusRoundIdentifier futureRoundId = new ConsensusRoundIdentifier(1, 5);
+  private final RoundSpecificNodeRoles futureRoles = context.getRoundSpecificRoles(futureRoundId);
+
+  private final MessageFactory localNodeMessageFactory = context.getLocalNodeMessageFactory();
+
+  @Before
+  public void setup() {
+    context.getController().start();
+  }
+
+  @Test
+  public void messagesForFutureRoundAreNotActionedUntilRoundIsActive() {
+    final Block futureBlock = context.createBlockForProposal(futureRoundId.getRoundNumber(), 60);
+    final int quorum = IbftHelpers.calculateRequiredValidatorQuorum(NETWORK_SIZE);
+    final ConsensusRoundIdentifier subsequentRoundId = new ConsensusRoundIdentifier(1, 6);
+    final RoundSpecificNodeRoles subsequentRoles = context.getRoundSpecificRoles(subsequentRoundId);
+
+    // required remotely received Prepares = quorum-2
+    // required remote received commits = quorum-1
+
+    // Inject 1 too few Commit messages (but sufficient Prepare
+    for (int i = 0; i < quorum - 3; i++) {
+      futureRoles.getNonProposingPeer(i).injectPrepare(futureRoundId, futureBlock.getHash());
+    }
+
+    for (int i = 0; i < quorum - 2; i++) {
+      futureRoles.getNonProposingPeer(i).injectCommit(futureRoundId, futureBlock.getHash());
+    }
+
+    // inject a prepare and a commit from a subsequent round, and ensure no transmissions are
+    // created
+    subsequentRoles.getNonProposingPeer(1).injectPrepare(subsequentRoundId, futureBlock.getHash());
+    subsequentRoles.getNonProposingPeer(1).injectCommit(subsequentRoundId, futureBlock.getHash());
+
+    assertPeersReceivedNoMessages(roles.getAllPeers());
+    assertThat(context.getBlockchain().getChainHeadBlockNumber()).isEqualTo(0);
+
+    // inject a newRound to move to 'futureRoundId', and ensure localnode sends prepare, commit
+    // and updates blockchain
+    injectEmptyNewRound(
+        futureRoundId, futureRoles.getProposer(), futureRoles.getAllPeers(), futureBlock);
+
+    final SignedData<PreparePayload> expectedPrepare =
+        localNodeMessageFactory.createSignedPreparePayload(futureRoundId, futureBlock.getHash());
+
+    assertPeersReceivedExactly(futureRoles.getAllPeers(), expectedPrepare);
+
+    // following 1 more prepare, a commit msg will be sent
+    futureRoles.getNonProposingPeer(quorum - 3).injectPrepare(futureRoundId, futureBlock.getHash());
+
+    final SignedData<CommitPayload> expectedCommit =
+        localNodeMessageFactory.createSignedCommitPayload(
+            futureRoundId,
+            futureBlock.getHash(),
+            SECP256K1.sign(futureBlock.getHash(), context.getLocalNodeParams().getNodeKeyPair()));
+    assertPeersReceivedExactly(futureRoles.getAllPeers(), expectedCommit);
+
+    // requires 1 more commit and the blockchain will progress
+    futureRoles.getNonProposingPeer(quorum - 2).injectCommit(futureRoundId, futureBlock.getHash());
+
+    assertThat(context.getBlockchain().getChainHeadBlockNumber()).isEqualTo(1);
+  }
+
+  @Test
+  public void priorRoundsCannotBeCompletedAfterReceptionOfNewRound() {
+    final Block initialBlock = context.createBlockForProposal(roundId.getRoundNumber(), 30);
+    final Block futureBlock = context.createBlockForProposal(futureRoundId.getRoundNumber(), 60);
+
+    roles.getProposer().injectProposal(roundId, initialBlock);
+    for (final ValidatorPeer peer : roles.getNonProposingPeers()) {
+      peer.injectPrepare(roundId, initialBlock.getHash());
+    }
+    roles.getProposer().injectCommit(roundId, initialBlock.getHash());
+    // At this stage, the local node has 2 commit msgs (proposer and local) so has not committed
+    assertThat(context.getBlockchain().getChainHeadBlockNumber()).isEqualTo(0);
+
+    for (final ValidatorPeer peer : roles.getAllPeers()) {
+      peer.clearReceivedMessages();
+    }
+
+    injectEmptyNewRound(
+        futureRoundId, futureRoles.getProposer(), futureRoles.getAllPeers(), futureBlock);
+
+    final SignedData<PreparePayload> expectedFuturePrepare =
+        localNodeMessageFactory.createSignedPreparePayload(futureRoundId, futureBlock.getHash());
+    assertPeersReceivedExactly(roles.getAllPeers(), expectedFuturePrepare);
+
+    // attempt to complete the previous round
+    roles.getNonProposingPeers().get(0).injectCommit(roundId, initialBlock.getHash());
+    assertPeersReceivedNoMessages(roles.getAllPeers());
+    assertThat(context.getBlockchain().getChainHeadBlockNumber()).isEqualTo(0);
+  }
+}

consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/ReceivedNewRoundTest.java

@@ -0,0 +1,231 @@
+/*
+ * Copyright 2019 ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+package tech.pegasys.pantheon.consensus.ibft.tests;
+
+import static java.util.Optional.empty;
+import static tech.pegasys.pantheon.consensus.ibft.support.MessageReceptionHelpers.assertPeersReceivedExactly;
+import static tech.pegasys.pantheon.consensus.ibft.support.MessageReceptionHelpers.assertPeersReceivedNoMessages;
+import static tech.pegasys.pantheon.consensus.ibft.support.TestHelpers.createValidPreparedCertificate;
+
+import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.CommitPayload;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.MessageFactory;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparePayload;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparedCertificate;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangeCertificate;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangePayload;
+import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData;
+import tech.pegasys.pantheon.consensus.ibft.support.RoundSpecificNodeRoles;
+import tech.pegasys.pantheon.consensus.ibft.support.TestContext;
+import tech.pegasys.pantheon.consensus.ibft.support.TestContextFactory;
+import tech.pegasys.pantheon.consensus.ibft.support.TestHelpers;
+import tech.pegasys.pantheon.consensus.ibft.support.ValidatorPeer;
+import tech.pegasys.pantheon.ethereum.core.Block;
+
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+import org.assertj.core.util.Lists;
+import org.junit.Before;
+import org.junit.Test;
+
+/** Ensure the Ibft component responds appropriately when a NewRound message is received. */
+public class ReceivedNewRoundTest {
+
+  private final TestContext context = TestContextFactory.createTestEnvWithArbitraryClock(5, 0);
+  private final ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(1, 0);
+  private final RoundSpecificNodeRoles roles = context.getRoundSpecificRoles(roundId);
+
+  private final MessageFactory localNodeMessageFactory = context.getLocalNodeMessageFactory();
+
+  @Before
+  public void setup() {
+    context.getController().start();
+  }
+
+  @Test
+  public void newRoundMessageWithEmptyPrepareCertificatesOfferNewBlock() {
+    final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
+    final Block blockToPropose = context.createBlockForProposal(nextRoundId.getRoundNumber(), 15);
+    final ConsensusRoundIdentifier targetRound = new ConsensusRoundIdentifier(1, 1);
+
+    final List<SignedData<RoundChangePayload>> roundChanges =
+        roles
+            .getAllPeers()
+            .stream()
+            .map(p -> p.getMessageFactory().createSignedRoundChangePayload(targetRound, empty()))
+            .collect(Collectors.toList());
+
+    final ValidatorPeer nextProposer = context.getRoundSpecificRoles(nextRoundId).getProposer();
+
+    nextProposer.injectNewRound(
+        targetRound,
+        new RoundChangeCertificate(roundChanges),
+        nextProposer.getMessageFactory().createSignedProposalPayload(targetRound, blockToPropose));
+
+    final SignedData<PreparePayload> expectedPrepare =
+        localNodeMessageFactory.createSignedPreparePayload(targetRound, blockToPropose.getHash());
+
+    assertPeersReceivedExactly(roles.getAllPeers(), expectedPrepare);
+  }
+
+  @Test
+  public void newRoundMessageFromIllegalSenderIsDiscardedAndNoPrepareForNewRoundIsSent() {
+    final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
+    final Block blockToPropose = context.createBlockForProposal(nextRoundId.getRoundNumber(), 15);
+
+    final List<SignedData<RoundChangePayload>> roundChanges =
+        roles
+            .getAllPeers()
+            .stream()
+            .map(p -> p.getMessageFactory().createSignedRoundChangePayload(nextRoundId, empty()))
+            .collect(Collectors.toList());
+
+    final ValidatorPeer illegalProposer =
+        context.getRoundSpecificRoles(nextRoundId).getNonProposingPeer(0);
+
+    illegalProposer.injectNewRound(
+        nextRoundId,
+        new RoundChangeCertificate(roundChanges),
+        illegalProposer
+            .getMessageFactory()
+            .createSignedProposalPayload(nextRoundId, blockToPropose));
+
+    assertPeersReceivedNoMessages(roles.getAllPeers());
+  }
+
+  @Test
+  public void newRoundWithPrepareCertificateResultsInNewRoundStartingWithExpectedBlock() {
+    final Block initialBlock = context.createBlockForProposal(0, 15);
+    final Block reproposedBlock = context.createBlockForProposal(1, 15);
+    final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
+
+    final PreparedCertificate preparedCertificate =
+        createValidPreparedCertificate(context, roundId, initialBlock);
+
+    final List<SignedData<RoundChangePayload>> roundChanges =
+        roles
+            .getAllPeers()
+            .stream()
+            .map(
+                p ->
+                    p.getMessageFactory()
+                        .createSignedRoundChangePayload(
+                            nextRoundId, Optional.of(preparedCertificate)))
+            .collect(Collectors.toList());
+
+    final ValidatorPeer nextProposer = context.getRoundSpecificRoles(nextRoundId).getProposer();
+
+    nextProposer.injectNewRound(
+        nextRoundId,
+        new RoundChangeCertificate(roundChanges),
+        roles
+            .getNonProposingPeer(0)
+            .getMessageFactory()
+            .createSignedProposalPayload(nextRoundId, reproposedBlock));
+
+    assertPeersReceivedExactly(
+        roles.getAllPeers(),
+        localNodeMessageFactory.createSignedPreparePayload(nextRoundId, reproposedBlock.getHash()));
+  }
+
+  @Test
+  public void newRoundMessageForPriorRoundIsNotActioned() {
+    // first move to a future round, then inject a newRound for a prior round, local node
+    // should send no messages.
+    final ConsensusRoundIdentifier futureRound = new ConsensusRoundIdentifier(1, 2);
+    for (final ValidatorPeer peer : roles.getAllPeers()) {
+      peer.injectRoundChange(futureRound, empty());
+    }
+
+    final ConsensusRoundIdentifier interimRound = new ConsensusRoundIdentifier(1, 1);
+    final List<SignedData<RoundChangePayload>> roundChangePayloads = Lists.newArrayList();
+    for (final ValidatorPeer peer : roles.getAllPeers()) {
+      roundChangePayloads.add(
+          peer.getMessageFactory().createSignedRoundChangePayload(interimRound, empty()));
+    }
+
+    final ValidatorPeer interimRoundProposer =
+        context.getRoundSpecificRoles(interimRound).getProposer();
+
+    final SignedData<ProposalPayload> proposal =
+        interimRoundProposer
+            .getMessageFactory()
+            .createSignedProposalPayload(interimRound, context.createBlockForProposal(1, 30));
+
+    interimRoundProposer.injectNewRound(
+        interimRound, new RoundChangeCertificate(roundChangePayloads), proposal);
+
+    assertPeersReceivedNoMessages(roles.getAllPeers());
+  }
+
+  @Test
+  public void receiveRoundStateIsNotLostIfASecondNewRoundMessageIsReceivedForCurrentRound() {
+    final Block initialBlock = context.createBlockForProposal(0, 15);
+    final Block reproposedBlock = context.createBlockForProposal(1, 15);
+    final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
+
+    final PreparedCertificate preparedCertificate =
+        createValidPreparedCertificate(context, roundId, initialBlock);
+
+    final List<SignedData<RoundChangePayload>> roundChanges =
+        roles
+            .getAllPeers()
+            .stream()
+            .map(
+                p ->
+                    p.getMessageFactory()
+                        .createSignedRoundChangePayload(
+                            nextRoundId, Optional.of(preparedCertificate)))
+            .collect(Collectors.toList());
+
+    final RoundSpecificNodeRoles nextRoles = context.getRoundSpecificRoles(nextRoundId);
+    final ValidatorPeer nextProposer = nextRoles.getProposer();
+
+    nextProposer.injectNewRound(
+        nextRoundId,
+        new RoundChangeCertificate(roundChanges),
+        roles
+            .getNonProposingPeer(0)
+            .getMessageFactory()
+            .createSignedProposalPayload(nextRoundId, reproposedBlock));
+
+    assertPeersReceivedExactly(
+        roles.getAllPeers(),
+        localNodeMessageFactory.createSignedPreparePayload(nextRoundId, reproposedBlock.getHash()));
+
+    // Inject a prepare, then re-inject the newRound - then ensure only a single prepare is enough
+    // to trigger a Commit transmission from the local node
+    nextRoles.getNonProposingPeer(0).injectPrepare(nextRoundId, reproposedBlock.getHash());
+
+    nextProposer.injectNewRound(
+        nextRoundId,
+        new RoundChangeCertificate(roundChanges),
+        roles
+            .getNonProposingPeer(0)
+            .getMessageFactory()
+            .createSignedProposalPayload(nextRoundId, reproposedBlock));
+
+    assertPeersReceivedNoMessages(roles.getAllPeers());
+
+    nextRoles.getNonProposingPeer(1).injectPrepare(nextRoundId, reproposedBlock.getHash());
+
+    final SignedData<CommitPayload> expectedCommit =
+        TestHelpers.createSignedCommentPayload(
+            reproposedBlock, context.getLocalNodeParams().getNodeKeyPair(), nextRoundId);
+
+    assertPeersReceivedExactly(nextRoles.getAllPeers(), expectedCommit);
+  }
+}

consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/RoundChangeTest.java

@@ -12,9 +12,11 @@
  */
 package tech.pegasys.pantheon.consensus.ibft.tests;
 
+import static java.util.Collections.emptyList;
 import static java.util.Optional.empty;
 import static tech.pegasys.pantheon.consensus.ibft.support.MessageReceptionHelpers.assertPeersReceivedExactly;
 import static tech.pegasys.pantheon.consensus.ibft.support.MessageReceptionHelpers.assertPeersReceivedNoMessages;
+import static tech.pegasys.pantheon.consensus.ibft.support.TestHelpers.createValidPreparedCertificate;
 
 import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
 import tech.pegasys.pantheon.consensus.ibft.IbftHelpers;
@@ -164,12 +166,14 @@ public class RoundChangeTest {
     final long ARBITRARY_BLOCKTIME = 1500;
 
     final PreparedCertificate earlierPrepCert =
-        createValidPrepCert(
+        createValidPreparedCertificate(
+            context,
             new ConsensusRoundIdentifier(1, 1),
             context.createBlockForProposal(1, ARBITRARY_BLOCKTIME / 2));
 
     final PreparedCertificate bestPrepCert =
-        createValidPrepCert(
+        createValidPreparedCertificate(
+            context,
             new ConsensusRoundIdentifier(1, 2),
             context.createBlockForProposal(2, ARBITRARY_BLOCKTIME));
 
@@ -253,7 +257,8 @@ public class RoundChangeTest {
     final ConsensusRoundIdentifier targetRound = new ConsensusRoundIdentifier(1, 4);
 
     final PreparedCertificate prepCert =
-        createValidPrepCert(
+        createValidPreparedCertificate(
+            context,
             new ConsensusRoundIdentifier(1, 2),
             context.createBlockForProposal(2, ARBITRARY_BLOCKTIME));
 
@@ -303,19 +308,38 @@ public class RoundChangeTest {
     assertPeersReceivedNoMessages(roles.getAllPeers());
   }
 
-  private PreparedCertificate createValidPrepCert(
+  @Test
-      final ConsensusRoundIdentifier preparedRound, final Block block) {
+  public void roundChangeExpiryForNonCurrentRoundIsDiscarded() {
-    final RoundSpecificNodeRoles roles = context.getRoundSpecificRoles(preparedRound);
+    // Manually timeout a future round, and ensure no messages are sent
+    context.getController().handleRoundExpiry(new RoundExpiry(new ConsensusRoundIdentifier(1, 1)));
+    assertPeersReceivedNoMessages(roles.getAllPeers());
+  }
+
+  @Test
+  public void illegallyConstructedRoundChangeMessageIsDiscarded() {
+    final ConsensusRoundIdentifier targetRound = new ConsensusRoundIdentifier(1, 4);
 
-    return new PreparedCertificate(
+    final SignedData<RoundChangePayload> rc1 =
-        roles.getProposer().getMessageFactory().createSignedProposalPayload(preparedRound, block),
+        roles.getNonProposingPeer(0).injectRoundChange(targetRound, empty());
+    final SignedData<RoundChangePayload> rc2 =
+        roles.getNonProposingPeer(1).injectRoundChange(targetRound, empty());
+    final SignedData<RoundChangePayload> rc3 =
+        roles.getNonProposingPeer(2).injectRoundChange(targetRound, empty());
+
+    // create illegal RoundChangeMessage
+    final PreparedCertificate illegalPreparedCertificate =
+        new PreparedCertificate(
             roles
-            .getNonProposingPeers()
+                .getNonProposingPeer(0)
-            .stream()
+                .getMessageFactory()
-            .map(
+                .createSignedProposalPayload(roundId, blockToPropose),
-                role ->
+            emptyList());
-                    role.getMessageFactory()
+
-                        .createSignedPreparePayload(preparedRound, block.getHash()))
+    roles
-            .collect(Collectors.toList()));
+        .getNonProposingPeer(2)
+        .injectRoundChange(targetRound, Optional.of(illegalPreparedCertificate));
+
+    // Ensure no NewRound message is sent.
+    assertPeersReceivedNoMessages(roles.getAllPeers());
   }
 }

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

@@ -37,6 +37,7 @@ import tech.pegasys.pantheon.ethereum.core.BlockHeader;
 import java.time.Clock;
 import java.util.Map;
 import java.util.Optional;
+import java.util.concurrent.TimeUnit;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.Function;
@@ -125,7 +126,7 @@ public class IbftBlockHeightManager {
 
   public void roundExpired(final RoundExpiry expire) {
     if (!expire.getView().equals(currentRound.getRoundIdentifier())) {
-      LOG.debug("Ignoring Round timer expired which does not match current round.");
+      LOG.info("Ignoring Round timer expired which does not match current round.");
       return;
     }
 
@@ -200,7 +201,7 @@ public class IbftBlockHeightManager {
       }
 
       if (finalState.isLocalNodeProposerForRound(targetRound)) {
-        currentRound.startRoundWith(result.get(), clock.millis() / 1000);
+        currentRound.startRoundWith(result.get(), TimeUnit.MILLISECONDS.toSeconds(clock.millis()));
       }
     }
   }