Remove start functionality from IbftController and IbftBlockHeightManager (#952)

Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
6 years ago · 3fee3a0da5
parent 03f367042f
commit 3fee3a0da5
18 changed files with 82 additions and 108 deletions
--- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/FutureHeightTest.java
+++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/FutureHeightTest.java
@ -31,7 +31,6 @@ import java.time.Clock;
 import java.time.Instant;
 import java.time.ZoneId;

-import org.junit.Before;
 import org.junit.Test;

 public class FutureHeightTest {
@ -57,11 +56,6 @@ public class FutureHeightTest {

  private final MessageFactory localNodeMessageFactory = context.getLocalNodeMessageFactory();

-  @Before
-  public void setup() {
-    context.getController().start();
-  }
-
  @Test
  public void messagesForFutureHeightAreBufferedUntilChainHeightCatchesUp() {
    final Block currentHeightBlock = context.createBlockForProposalFromChainHead(0, 30);
--- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/FutureRoundTest.java
+++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/FutureRoundTest.java
@ -30,7 +30,6 @@ import java.time.Clock;
 import java.time.Instant;
 import java.time.ZoneId;

-import org.junit.Before;
 import org.junit.Test;

 public class FutureRoundTest {
@ -57,11 +56,6 @@ public class FutureRoundTest {

  private final MessageFactory localNodeMessageFactory = context.getLocalNodeMessageFactory();

-  @Before
-  public void setup() {
-    context.getController().start();
-  }
-
  @Test
  public void messagesForFutureRoundAreNotActionedUntilRoundIsActive() {
    final Block futureBlock =
--- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/GossipTest.java
+++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/GossipTest.java
@ -64,7 +64,6 @@ public class GossipTest {

  @Before
  public void setup() {
-    context.getController().start();
    block = context.createBlockForProposalFromChainHead(roundId.getRoundNumber(), 30);
    sender = peers.getProposer();
    msgFactory = sender.getMessageFactory();
--- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/LocalNodeIsProposerTest.java
+++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/LocalNodeIsProposerTest.java
@ -72,8 +72,7 @@ public class LocalNodeIsProposerTest {
            createSignedCommitPayload(
                roundId, expectedProposedBlock, context.getLocalNodeParams().getNodeKeyPair()));

-    // Start the Controller, and trigger "block timer" to send proposal.
-    context.getController().start();
+    // Trigger "block timer" to send proposal.
    context.getController().handleBlockTimerExpiry(new BlockTimerExpiry(roundId));
  }

--- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/LocalNodeNotProposerTest.java
+++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/LocalNodeNotProposerTest.java
@ -56,8 +56,6 @@ public class LocalNodeNotProposerTest {
        new Commit(
            createSignedCommitPayload(
                roundId, blockToPropose, context.getLocalNodeParams().getNodeKeyPair()));
-
-    context.getController().start();
  }

  @Test
--- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/ReceivedFutureProposalTest.java
+++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/ReceivedFutureProposalTest.java
@ -31,7 +31,6 @@ import tech.pegasys.pantheon.ethereum.core.Block;

 import java.util.List;

-import org.junit.Before;
 import org.junit.Test;

 /**
@ -53,11 +52,6 @@ public class ReceivedFutureProposalTest {

  private final MessageFactory localNodeMessageFactory = context.getLocalNodeMessageFactory();

-  @Before
-  public void setup() {
-    context.getController().start();
-  }
-
  @Test
  public void proposalWithEmptyPrepareCertificatesOfferNewBlock() {
    final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
--- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/RoundChangeTest.java
+++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/RoundChangeTest.java
@ -40,7 +40,6 @@ import java.util.List;
 import java.util.Optional;

 import com.google.common.collect.Lists;
-import org.junit.Before;
 import org.junit.Test;

 public class RoundChangeTest {
@ -65,11 +64,6 @@ public class RoundChangeTest {

  private final Block blockToPropose = context.createBlockForProposalFromChainHead(0, 15);

-  @Before
-  public void setup() {
-    context.getController().start();
-  }
-
  @Test
  public void onRoundChangeTimerExpiryEventRoundChangeMessageIsSent() {

--- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/SpuriousBehaviourTest.java
+++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/SpuriousBehaviourTest.java
@ -67,7 +67,6 @@ public class SpuriousBehaviourTest {

  @Before
  public void setup() {
-    context.getController().start();

    expectedPrepare =
        context.getLocalNodeMessageFactory().createPrepare(roundId, proposedBlock.getHash());
--- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/BlockHeightManager.java
+++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/BlockHeightManager.java
@ -22,8 +22,6 @@ import tech.pegasys.pantheon.ethereum.core.BlockHeader;

 public interface BlockHeightManager {

-  void start();
-
  void handleBlockTimerExpiry(ConsensusRoundIdentifier roundIdentifier);

  void roundExpired(RoundExpiry expire);
--- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java
+++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java
@ -18,7 +18,6 @@ import static tech.pegasys.pantheon.consensus.ibft.statemachine.IbftBlockHeightM

 import tech.pegasys.pantheon.consensus.ibft.BlockTimer;
 import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
-import tech.pegasys.pantheon.consensus.ibft.RoundTimer;
 import tech.pegasys.pantheon.consensus.ibft.ibftevent.RoundExpiry;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Commit;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.IbftMessage;
@ -58,7 +57,6 @@ public class IbftBlockHeightManager implements BlockHeightManager {
  private final IbftRoundFactory roundFactory;
  private final RoundChangeManager roundChangeManager;
  private final BlockHeader parentHeader;
-  private final RoundTimer roundTimer;
  private final BlockTimer blockTimer;
  private final IbftMessageTransmitter transmitter;
  private final MessageFactory messageFactory;
@ -81,7 +79,6 @@ public class IbftBlockHeightManager implements BlockHeightManager {
      final MessageValidatorFactory messageValidatorFactory) {
    this.parentHeader = parentHeader;
    this.roundFactory = ibftRoundFactory;
-    this.roundTimer = finalState.getRoundTimer();
    this.blockTimer = finalState.getBlockTimer();
    this.transmitter = finalState.getTransmitter();
    this.messageFactory = finalState.getMessageFactory();
@ -99,11 +96,8 @@ public class IbftBlockHeightManager implements BlockHeightManager {
                roundIdentifier,
                finalState.getQuorum(),
                messageValidatorFactory.createMessageValidator(roundIdentifier, parentHeader));
-  }

-  @Override
-  public void start() {
-    startNewRound(0);
+    currentRound = roundFactory.createNewRound(parentHeader, 0);
    if (finalState.isLocalNodeProposerForRound(currentRound.getRoundIdentifier())) {
      blockTimer.startTimer(currentRound.getRoundIdentifier(), parentHeader);
    }
@ -247,7 +241,6 @@ public class IbftBlockHeightManager implements BlockHeightManager {
    }
    // discard roundChange messages from the current and previous rounds
    roundChangeManager.discardRoundsPriorTo(currentRound.getRoundIdentifier());
-    roundTimer.startTimer(currentRound.getRoundIdentifier());
  }

  @Override
--- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java
+++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java
@ -40,7 +40,6 @@ import org.apache.logging.log4j.Logger;
 public class IbftController {

  private static final Logger LOG = LogManager.getLogger();
-  private final Blockchain blockchain;
  private final IbftFinalState ibftFinalState;
  private final IbftBlockHeightManagerFactory ibftBlockHeightManagerFactory;
  private final FutureMessageBuffer futureMessageBuffer;
@ -57,16 +56,13 @@ public class IbftController {
      final MessageTracker duplicateMessageTracker,
      final FutureMessageBuffer futureMessageBuffer,
      final SynchronizerUpdater sychronizerUpdater) {
-    this.blockchain = blockchain;
    this.ibftFinalState = ibftFinalState;
    this.ibftBlockHeightManagerFactory = ibftBlockHeightManagerFactory;
    this.futureMessageBuffer = futureMessageBuffer;
    this.gossiper = gossiper;
    this.duplicateMessageTracker = duplicateMessageTracker;
    this.sychronizerUpdater = sychronizerUpdater;
-  }

-  public void start() {
    startNewHeightManager(blockchain.getChainHeadHeader());
  }

@ -183,7 +179,6 @@ public class IbftController {

  private void startNewHeightManager(final BlockHeader parentHeader) {
    currentHeightManager = ibftBlockHeightManagerFactory.create(parentHeader);
-    currentHeightManager.start();
    final long newChainHeight = currentHeightManager.getChainHeight();
    futureMessageBuffer.retrieveMessagesForHeight(newChainHeight).forEach(this::handleMessage);
  }
--- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRound.java
+++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRound.java
@ -18,6 +18,7 @@ import tech.pegasys.pantheon.consensus.ibft.IbftBlockInterface;
 import tech.pegasys.pantheon.consensus.ibft.IbftContext;
 import tech.pegasys.pantheon.consensus.ibft.IbftExtraData;
 import tech.pegasys.pantheon.consensus.ibft.IbftHelpers;
+import tech.pegasys.pantheon.consensus.ibft.RoundTimer;
 import tech.pegasys.pantheon.consensus.ibft.blockcreation.IbftBlockCreator;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Commit;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Prepare;
@ -63,7 +64,8 @@ public class IbftRound {
      final Subscribers<MinedBlockObserver> observers,
      final KeyPair nodeKeys,
      final MessageFactory messageFactory,
-      final IbftMessageTransmitter transmitter) {
+      final IbftMessageTransmitter transmitter,
+      final RoundTimer roundTimer) {
    this.roundState = roundState;
    this.blockCreator = blockCreator;
    this.protocolContext = protocolContext;
@ -72,6 +74,8 @@ public class IbftRound {
    this.nodeKeys = nodeKeys;
    this.messageFactory = messageFactory;
    this.transmitter = transmitter;
+
+    roundTimer.startTimer(getRoundIdentifier());
  }

  public ConsensusRoundIdentifier getRoundIdentifier() {
--- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundFactory.java
+++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundFactory.java
@ -73,6 +73,7 @@ public class IbftRoundFactory {
        minedBlockObservers,
        finalState.getNodeKeys(),
        finalState.getMessageFactory(),
-        finalState.getTransmitter());
+        finalState.getTransmitter(),
+        finalState.getRoundTimer());
  }
 }
--- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/NoOpBlockHeightManager.java
+++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/NoOpBlockHeightManager.java
@ -28,9 +28,6 @@ public class NoOpBlockHeightManager implements BlockHeightManager {
    this.parentHeader = parentHeader;
  }

-  @Override
-  public void start() {}
-
  @Override
  public void handleBlockTimerExpiry(final ConsensusRoundIdentifier roundIdentifier) {}

--- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java
+++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java
@ -131,7 +131,6 @@ public class IbftBlockHeightManagerTest {
    when(messageValidator.validatePrepare(any())).thenReturn(true);
    when(finalState.getTransmitter()).thenReturn(messageTransmitter);
    when(finalState.getBlockTimer()).thenReturn(blockTimer);
-    when(finalState.getRoundTimer()).thenReturn(roundTimer);
    when(finalState.getQuorum()).thenReturn(3);
    when(finalState.getMessageFactory()).thenReturn(messageFactory);
    when(blockCreator.createBlock(anyLong())).thenReturn(createdBlock);
@ -159,7 +158,8 @@ public class IbftBlockHeightManagerTest {
                  new Subscribers<>(),
                  localNodeKeys,
                  messageFactory,
-                  messageTransmitter);
+                  messageTransmitter,
+                  roundTimer);
            });

    when(roundFactory.createNewRoundWithState(any(), any()))
@ -174,7 +174,8 @@ public class IbftBlockHeightManagerTest {
                  new Subscribers<>(),
                  localNodeKeys,
                  messageFactory,
-                  messageTransmitter);
+                  messageTransmitter,
+                  roundTimer);
            });
  }

@ -190,7 +191,6 @@ public class IbftBlockHeightManagerTest {
            roundFactory,
            clock,
            messageValidatorFactory);
-    manager.start();

    verify(blockTimer, times(1)).startTimer(any(), any());
  }
@ -205,7 +205,6 @@ public class IbftBlockHeightManagerTest {
            roundFactory,
            clock,
            messageValidatorFactory);
-    manager.start();

    manager.handleBlockTimerExpiry(roundIdentifier);
    verify(messageTransmitter, times(1)).multicastProposal(eq(roundIdentifier), any(), any());
@ -230,7 +229,6 @@ public class IbftBlockHeightManagerTest {
            roundFactory,
            clock,
            messageValidatorFactory);
-    manager.start();
    verify(roundFactory).createNewRound(any(), eq(0));

    manager.handleRoundChangePayload(roundChange);
@ -250,7 +248,6 @@ public class IbftBlockHeightManagerTest {
            roundFactory,
            clock,
            messageValidatorFactory);
-    manager.start();
    verify(roundFactory).createNewRound(any(), eq(0));

    manager.roundExpired(new RoundExpiry(roundIdentifier));
@ -277,7 +274,6 @@ public class IbftBlockHeightManagerTest {
            roundFactory,
            clock,
            messageValidatorFactory);
-    manager.start();
    reset(messageTransmitter);

    manager.handleRoundChangePayload(roundChange);
@ -298,7 +294,6 @@ public class IbftBlockHeightManagerTest {
            roundFactory,
            clock,
            messageValidatorFactory);
-    manager.start();

    final Prepare prepare =
        validatorMessageFactory
@ -339,7 +334,6 @@ public class IbftBlockHeightManagerTest {
            roundFactory,
            clock,
            messageValidatorFactory);
-    manager.start();
    manager.handleBlockTimerExpiry(roundIdentifier); // Trigger a Proposal creation.

    final Prepare firstPrepare =
@ -388,7 +382,6 @@ public class IbftBlockHeightManagerTest {
            futureRoundIdentifier,
            createdBlock,
            Optional.of(new RoundChangeCertificate(Collections.emptyList())));
-    manager.start();
    reset(roundFactory); // Discard the existing createNewRound invocation.

    manager.handleProposalPayload(futureRoundProposal);
--- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftControllerTest.java
+++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftControllerTest.java
@ -96,15 +96,6 @@ public class IbftControllerTest {
    when(blockChain.getChainHeadHeader()).thenReturn(chainHeadBlockHeader);
    when(blockHeightManagerFactory.create(any())).thenReturn(blockHeightManager);
    when(ibftFinalState.getValidators()).thenReturn(ImmutableList.of(validator));
-    ibftController =
-        new IbftController(
-            blockChain,
-            ibftFinalState,
-            blockHeightManagerFactory,
-            ibftGossip,
-            messageTracker,
-            futureMessageBuffer,
-            mock(EthSynchronizerUpdater.class));

    when(chainHeadBlockHeader.getNumber()).thenReturn(1L);
    when(chainHeadBlockHeader.getHash()).thenReturn(Hash.ZERO);
@ -117,9 +108,21 @@ public class IbftControllerTest {
    when(messageTracker.hasSeenMessage(any())).thenReturn(false);
  }

+  private void constructIbftController() {
+    ibftController =
+        new IbftController(
+            blockChain,
+            ibftFinalState,
+            blockHeightManagerFactory,
+            ibftGossip,
+            messageTracker,
+            futureMessageBuffer,
+            mock(EthSynchronizerUpdater.class));
+  }
+
  @Test
  public void createsNewBlockHeightManagerWhenStarted() {
-    ibftController.start();
+    constructIbftController();
    verify(futureMessageBuffer, never()).addMessage(anyLong(), any());
    verify(blockHeightManagerFactory).create(chainHeadBlockHeader);
  }
@ -137,12 +140,12 @@ public class IbftControllerTest {
    when(blockHeightManager.getChainHeight()).thenReturn(2L);
    when(futureMessageBuffer.retrieveMessagesForHeight(2L)).thenReturn(height2Msgs);

-    ibftController.start();
+    constructIbftController();
+
    verify(futureMessageBuffer).retrieveMessagesForHeight(2L);
    verify(futureMessageBuffer, never()).retrieveMessagesForHeight(3L);
    verify(blockHeightManagerFactory).create(chainHeadBlockHeader);
    verify(blockHeightManager, atLeastOnce()).getChainHeight();
-    verify(blockHeightManager).start();
    verify(blockHeightManager, never()).handleProposalPayload(proposal);
    verify(blockHeightManager).handlePreparePayload(prepare);
    verify(ibftGossip).send(prepareMessage);
@ -165,13 +168,12 @@ public class IbftControllerTest {
        .thenReturn(emptyList());
    when(blockHeightManager.getChainHeight()).thenReturn(2L);

-    ibftController.start();
+    constructIbftController();
    final NewChainHead newChainHead = new NewChainHead(nextBlock);
    ibftController.handleNewBlockEvent(newChainHead);

    verify(blockHeightManagerFactory).create(nextBlock);
    verify(blockHeightManager, atLeastOnce()).getChainHeight();
-    verify(blockHeightManager, times(2)).start(); // once at beginning, and again on newChainHead.
    verify(futureMessageBuffer, times(2)).retrieveMessagesForHeight(2L);
    verify(blockHeightManager).handleProposalPayload(proposal);
    verify(ibftGossip).send(proposalMessage);
@ -185,28 +187,25 @@ public class IbftControllerTest {

  @Test
  public void newBlockForCurrentOrPreviousHeightTriggersNoChange() {
-    ibftController.start();
-
+    constructIbftController();
    long chainHeadHeight = chainHeadBlockHeader.getNumber();
    when(nextBlock.getNumber()).thenReturn(chainHeadHeight);
    when(nextBlock.getHash()).thenReturn(Hash.ZERO);
    final NewChainHead sameHeightBlock = new NewChainHead(nextBlock);
    ibftController.handleNewBlockEvent(sameHeightBlock);
    verify(blockHeightManagerFactory, times(1)).create(any()); // initial creation
-    verify(blockHeightManager, times(1)).start(); // the initial call at start of test.

    when(nextBlock.getNumber()).thenReturn(chainHeadHeight - 1);
    final NewChainHead priorBlock = new NewChainHead(nextBlock);
    ibftController.handleNewBlockEvent(priorBlock);
    verify(blockHeightManagerFactory, times(1)).create(any());
-    verify(blockHeightManager, times(1)).start();
  }

  @Test
  public void handlesRoundExpiry() {
    final RoundExpiry roundExpiry = new RoundExpiry(roundIdentifier);

-    ibftController.start();
+    constructIbftController();
    ibftController.handleRoundExpiry(roundExpiry);

    verify(blockHeightManager).roundExpired(roundExpiry);
@ -216,7 +215,7 @@ public class IbftControllerTest {
  public void handlesBlockTimerExpiry() {
    final BlockTimerExpiry blockTimerExpiry = new BlockTimerExpiry(roundIdentifier);

-    ibftController.start();
+    constructIbftController();
    ibftController.handleBlockTimerExpiry(blockTimerExpiry);

    verify(blockHeightManager).handleBlockTimerExpiry(roundIdentifier);
@ -225,42 +224,39 @@ public class IbftControllerTest {
  @Test
  public void proposalForCurrentHeightIsPassedToBlockHeightManager() {
    setupProposal(roundIdentifier, validator);
-    ibftController.start();
+    constructIbftController();
    ibftController.handleMessageEvent(new IbftReceivedMessageEvent(proposalMessage));

    verify(futureMessageBuffer, never()).addMessage(anyLong(), any());
    verify(blockHeightManager).handleProposalPayload(proposal);
    verify(ibftGossip).send(proposalMessage);
    verify(blockHeightManager, atLeastOnce()).getChainHeight();
-    verify(blockHeightManager).start();
    verifyNoMoreInteractions(blockHeightManager);
  }

  @Test
  public void prepareForCurrentHeightIsPassedToBlockHeightManager() {
    setupPrepare(roundIdentifier, validator);
-    ibftController.start();
+    constructIbftController();
    ibftController.handleMessageEvent(new IbftReceivedMessageEvent(prepareMessage));

    verify(futureMessageBuffer, never()).addMessage(anyLong(), any());
    verify(blockHeightManager).handlePreparePayload(prepare);
    verify(ibftGossip).send(prepareMessage);
    verify(blockHeightManager, atLeastOnce()).getChainHeight();
-    verify(blockHeightManager).start();
    verifyNoMoreInteractions(blockHeightManager);
  }

  @Test
  public void commitForCurrentHeightIsPassedToBlockHeightManager() {
    setupCommit(roundIdentifier, validator);
-    ibftController.start();
+    constructIbftController();
    ibftController.handleMessageEvent(new IbftReceivedMessageEvent(commitMessage));

    verify(futureMessageBuffer, never()).addMessage(anyLong(), any());
    verify(blockHeightManager).handleCommitPayload(commit);
    verify(ibftGossip).send(commitMessage);
    verify(blockHeightManager, atLeastOnce()).getChainHeight();
-    verify(blockHeightManager).start();
    verifyNoMoreInteractions(blockHeightManager);
  }

@ -268,14 +264,13 @@ public class IbftControllerTest {
  public void roundChangeForCurrentHeightIsPassedToBlockHeightManager() {
    roundIdentifier = new ConsensusRoundIdentifier(0, 1);
    setupRoundChange(roundIdentifier, validator);
-    ibftController.start();
+    constructIbftController();
    ibftController.handleMessageEvent(new IbftReceivedMessageEvent(roundChangeMessage));

    verify(futureMessageBuffer, never()).addMessage(anyLong(), any());
    verify(blockHeightManager).handleRoundChangePayload(roundChange);
    verify(ibftGossip).send(roundChangeMessage);
    verify(blockHeightManager, atLeastOnce()).getChainHeight();
-    verify(blockHeightManager).start();
    verifyNoMoreInteractions(blockHeightManager);
  }

@ -311,7 +306,7 @@ public class IbftControllerTest {
  public void roundExpiryForPastHeightIsDiscarded() {
    final RoundExpiry roundExpiry = new RoundExpiry(roundIdentifier);
    when(blockHeightManager.getChainHeight()).thenReturn(1L);
-    ibftController.start();
+    constructIbftController();
    ibftController.handleRoundExpiry(roundExpiry);
    verify(futureMessageBuffer, never()).addMessage(anyLong(), any());
    verify(blockHeightManager, never()).roundExpired(any());
@ -321,7 +316,7 @@ public class IbftControllerTest {
  public void blockTimerForPastHeightIsDiscarded() {
    final BlockTimerExpiry blockTimerExpiry = new BlockTimerExpiry(roundIdentifier);
    when(blockHeightManager.getChainHeight()).thenReturn(1L);
-    ibftController.start();
+    constructIbftController();
    ibftController.handleBlockTimerExpiry(blockTimerExpiry);
    verify(futureMessageBuffer, never()).addMessage(anyLong(), any());
    verify(blockHeightManager, never()).handleBlockTimerExpiry(any());
@ -387,29 +382,27 @@ public class IbftControllerTest {
  public void uniqueMessagesAreAddedAsSeen() {
    when(messageTracker.hasSeenMessage(proposalMessageData)).thenReturn(false);
    setupProposal(roundIdentifier, validator);
-    ibftController.start();
+    constructIbftController();
    ibftController.handleMessageEvent(new IbftReceivedMessageEvent(proposalMessage));

    verify(messageTracker).addSeenMessage(proposalMessageData);
  }

  private void verifyNotHandledAndNoFutureMsgs(final IbftReceivedMessageEvent msg) {
-    ibftController.start();
+    constructIbftController();
    ibftController.handleMessageEvent(msg);

    verify(futureMessageBuffer, never()).addMessage(anyLong(), any());
    verify(blockHeightManager, atLeastOnce()).getChainHeight();
-    verify(blockHeightManager).start();
    verifyNoMoreInteractions(blockHeightManager);
  }

  private void verifyHasFutureMessages(final long msgHeight, final Message message) {
-    ibftController.start();
+    constructIbftController();
    ibftController.handleMessageEvent(new IbftReceivedMessageEvent(message));

    verify(futureMessageBuffer).addMessage(msgHeight, message);
    verify(blockHeightManager, atLeastOnce()).getChainHeight();
-    verify(blockHeightManager).start();
    verifyNoMoreInteractions(blockHeightManager);
  }

--- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundTest.java
+++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundTest.java
@ -28,6 +28,7 @@ import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
 import tech.pegasys.pantheon.consensus.ibft.IbftBlockHashing;
 import tech.pegasys.pantheon.consensus.ibft.IbftContext;
 import tech.pegasys.pantheon.consensus.ibft.IbftExtraData;
+import tech.pegasys.pantheon.consensus.ibft.RoundTimer;
 import tech.pegasys.pantheon.consensus.ibft.blockcreation.IbftBlockCreator;
 import tech.pegasys.pantheon.consensus.ibft.network.IbftMessageTransmitter;
 import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory;
@ -79,6 +80,7 @@ public class IbftRoundTest {
  @Mock private MinedBlockObserver minedBlockObserver;
  @Mock private IbftBlockCreator blockCreator;
  @Mock private MessageValidator messageValidator;
+  @Mock private RoundTimer roundTimer;

  @Captor private ArgumentCaptor<SignedData<ProposalPayload>> payloadArgCaptor;
  @Captor private ArgumentCaptor<Block> blockCaptor;
@ -115,6 +117,23 @@ public class IbftRoundTest {
    subscribers.subscribe(minedBlockObserver);
  }

+  @Test
+  public void onConstructionRoundTimerIsStarted() {
+    final RoundState roundState = new RoundState(roundIdentifier, 3, messageValidator);
+    final IbftRound round =
+        new IbftRound(
+            roundState,
+            blockCreator,
+            protocolContext,
+            blockImporter,
+            subscribers,
+            localNodeKeys,
+            messageFactory,
+            transmitter,
+            roundTimer);
+    verify(roundTimer, times(1)).startTimer(roundIdentifier);
+  }
+
  @Test
  public void onReceptionOfValidProposalSendsAPrepareToNetworkPeers() {
    final RoundState roundState = new RoundState(roundIdentifier, 3, messageValidator);
@ -127,7 +146,8 @@ public class IbftRoundTest {
            subscribers,
            localNodeKeys,
            messageFactory,
-            transmitter);
+            transmitter,
+            roundTimer);

    round.handleProposalMessage(
        messageFactory.createProposal(roundIdentifier, proposedBlock, Optional.empty()));
@ -147,7 +167,8 @@ public class IbftRoundTest {
            subscribers,
            localNodeKeys,
            messageFactory,
-            transmitter);
+            transmitter,
+            roundTimer);

    round.createAndSendProposalMessage(15);
    verify(transmitter, times(1))
@ -168,7 +189,8 @@ public class IbftRoundTest {
            subscribers,
            localNodeKeys,
            messageFactory,
-            transmitter);
+            transmitter,
+            roundTimer);
    round.createAndSendProposalMessage(15);
    verify(transmitter, times(1))
        .multicastProposal(roundIdentifier, proposedBlock, Optional.empty());
@ -189,7 +211,8 @@ public class IbftRoundTest {
            subscribers,
            localNodeKeys,
            messageFactory,
-            transmitter);
+            transmitter,
+            roundTimer);

    final Hash commitSealHash =
        IbftBlockHashing.calculateDataHashForCommittedSeal(
@ -231,7 +254,8 @@ public class IbftRoundTest {
            subscribers,
            localNodeKeys,
            messageFactory,
-            transmitter);
+            transmitter,
+            roundTimer);

    final Hash commitSealHash =
        IbftBlockHashing.calculateDataHashForCommittedSeal(
@ -266,7 +290,8 @@ public class IbftRoundTest {
            subscribers,
            localNodeKeys,
            messageFactory,
-            transmitter);
+            transmitter,
+            roundTimer);

    final RoundChangeCertificate roundChangeCertificate = new RoundChangeCertificate(emptyList());

@ -288,7 +313,8 @@ public class IbftRoundTest {
            subscribers,
            localNodeKeys,
            messageFactory,
-            transmitter);
+            transmitter,
+            roundTimer);

    final RoundChangeArtifacts roundChangeArtifacts =
        RoundChangeArtifacts.create(
@ -333,7 +359,8 @@ public class IbftRoundTest {
            subscribers,
            localNodeKeys,
            messageFactory,
-            transmitter);
+            transmitter,
+            roundTimer);

    final RoundChangeArtifacts roundChangeArtifacts =
        RoundChangeArtifacts.create(
@ -365,7 +392,8 @@ public class IbftRoundTest {
            subscribers,
            localNodeKeys,
            messageFactory,
-            transmitter);
+            transmitter,
+            roundTimer);
    round.createAndSendProposalMessage(15);
    verify(minedBlockObserver).blockMined(any());
  }
@ -384,7 +412,8 @@ public class IbftRoundTest {
            subscribers,
            localNodeKeys,
            messageFactory,
-            transmitter);
+            transmitter,
+            roundTimer);

    round.handleCommitMessage(
        messageFactory.createCommit(roundIdentifier, proposedBlock.getHash(), remoteCommitSeal));
@ -408,7 +437,8 @@ public class IbftRoundTest {
            subscribers,
            localNodeKeys,
            messageFactory,
-            transmitter);
+            transmitter,
+            roundTimer);

    round.handleCommitMessage(
        messageFactory.createCommit(roundIdentifier, proposedBlock.getHash(), remoteCommitSeal));
--- a/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java
+++ b/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java
@ -265,7 +265,6 @@ public class IbftPantheonController implements PantheonController<IbftContext> {
            duplicateMessageTracker,
            futureMessageBuffer,
            new EthSynchronizerUpdater(ethContext.getEthPeers()));
-    ibftController.start();

    final EventMultiplexer eventMultiplexer = new EventMultiplexer(ibftController);
    final IbftProcessor ibftProcessor = new IbftProcessor(ibftEventQueue, eventMultiplexer);