Separate round change reception from RoundChangeCertificate

consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockInterface.java

@@ -16,7 +16,10 @@
 import tech.pegasys.pantheon.consensus.common.ValidatorVote;
 import tech.pegasys.pantheon.consensus.common.VoteType;
 import tech.pegasys.pantheon.ethereum.core.Address;
+import tech.pegasys.pantheon.ethereum.core.Block;
+import tech.pegasys.pantheon.ethereum.core.BlockHashFunction;
 import tech.pegasys.pantheon.ethereum.core.BlockHeader;
+import tech.pegasys.pantheon.ethereum.core.BlockHeaderBuilder;
 
 import java.util.Collection;
 import java.util.Optional;
@@ -49,4 +52,23 @@ public Collection<Address> validatorsInBlock(final BlockHeader header) {
     final IbftExtraData ibftExtraData = IbftExtraData.decode(header.getExtraData());
     return ibftExtraData.getValidators();
   }
+
+  public static Block replaceRoundInBlock(
+      final Block block, final int round, final BlockHashFunction blockHashFunction) {
+    final IbftExtraData prevExtraData = IbftExtraData.decode(block.getHeader().getExtraData());
+    final IbftExtraData substituteExtraData =
+        new IbftExtraData(
+            prevExtraData.getVanityData(),
+            prevExtraData.getSeals(),
+            prevExtraData.getVote(),
+            round,
+            prevExtraData.getValidators());
+
+    final BlockHeaderBuilder headerBuilder = BlockHeaderBuilder.fromHeader(block.getHeader());
+    headerBuilder.extraData(substituteExtraData.encode()).blockHashFunction(blockHashFunction);
+
+    final BlockHeader newHeader = headerBuilder.buildBlockHeader();
+
+    return new Block(newHeader, block.getBody());
+  }
 }

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

@@ -29,12 +29,12 @@
 import tech.pegasys.pantheon.consensus.ibft.network.IbftMessageTransmitter;
 import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory;
 import tech.pegasys.pantheon.consensus.ibft.payload.Payload;
-import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangeCertificate;
 import tech.pegasys.pantheon.consensus.ibft.validation.MessageValidatorFactory;
 import tech.pegasys.pantheon.consensus.ibft.validation.NewRoundMessageValidator;
 import tech.pegasys.pantheon.ethereum.core.BlockHeader;
 
 import java.time.Clock;
+import java.util.Collection;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.TimeUnit;
@@ -202,15 +202,18 @@ public void handleRoundChangePayload(final RoundChange message) {
       return;
     }
 
-    final Optional<RoundChangeCertificate> result =
+    final Optional<Collection<RoundChange>> result =
         roundChangeManager.appendRoundChangeMessage(message);
     if (result.isPresent()) {
       if (messageAge == FUTURE_ROUND) {
         startNewRound(targetRound.getRoundNumber());
       }
 
+      final RoundChangeArtefacts roundChangeArtefacts = RoundChangeArtefacts.create(result.get());
+
       if (finalState.isLocalNodeProposerForRound(targetRound)) {
-        currentRound.startRoundWith(result.get(), TimeUnit.MILLISECONDS.toSeconds(clock.millis()));
+        currentRound.startRoundWith(
+            roundChangeArtefacts, TimeUnit.MILLISECONDS.toSeconds(clock.millis()));
       }
     }
   }

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

@@ -12,10 +12,9 @@
  */
 package tech.pegasys.pantheon.consensus.ibft.statemachine;
 
-import static tech.pegasys.pantheon.consensus.ibft.IbftHelpers.findLatestPreparedCertificate;
-
 import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
 import tech.pegasys.pantheon.consensus.ibft.IbftBlockHashing;
+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;
@@ -26,16 +25,13 @@
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Proposal;
 import tech.pegasys.pantheon.consensus.ibft.network.IbftMessageTransmitter;
 import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory;
-import tech.pegasys.pantheon.consensus.ibft.payload.PreparedCertificate;
-import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangeCertificate;
 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.ProtocolContext;
 import tech.pegasys.pantheon.ethereum.chain.MinedBlockObserver;
 import tech.pegasys.pantheon.ethereum.core.Block;
 import tech.pegasys.pantheon.ethereum.core.BlockHeader;
-import tech.pegasys.pantheon.ethereum.core.BlockHeaderBuilder;
 import tech.pegasys.pantheon.ethereum.core.BlockImporter;
 import tech.pegasys.pantheon.ethereum.core.Hash;
 import tech.pegasys.pantheon.ethereum.mainnet.HeaderValidationMode;
@@ -97,53 +93,34 @@ public void createAndSendProposalMessage(final long headerTimeStampSeconds) {
   }
 
   public void startRoundWith(
-      final RoundChangeCertificate roundChangeCertificate, final long headerTimestamp) {
-    final Optional<PreparedCertificate> latestCertificate =
-        findLatestPreparedCertificate(roundChangeCertificate.getRoundChangePayloads());
+      final RoundChangeArtefacts roundChangeArtefacts, final long headerTimestamp) {
+    final Optional<Block> bestBlockFromRoundChange = roundChangeArtefacts.getBlock();
 
     Proposal proposal;
-    if (!latestCertificate.isPresent()) {
+    if (!bestBlockFromRoundChange.isPresent()) {
       LOG.trace("Multicasting NewRound with new block. round={}", roundState.getRoundIdentifier());
       final Block block = blockCreator.createBlock(headerTimestamp);
       proposal = messageFactory.createSignedProposalPayload(getRoundIdentifier(), block);
     } else {
       LOG.trace(
           "Multicasting NewRound from PreparedCertificate. round={}",
           roundState.getRoundIdentifier());
-      proposal = createProposalFromPreparedCertificate(latestCertificate.get());
+      proposal = createProposalAroundBlock(bestBlockFromRoundChange.get());
     }
     transmitter.multicastNewRound(
-        getRoundIdentifier(), roundChangeCertificate, proposal.getSignedPayload());
+        getRoundIdentifier(),
+        roundChangeArtefacts.getRoundChangeCertificate(),
+        proposal.getSignedPayload());
     updateStateWithProposedBlock(proposal);
   }
 
-  private Proposal createProposalFromPreparedCertificate(
-      final PreparedCertificate preparedCertificate) {
-    final Block block = preparedCertificate.getProposalPayload().getPayload().getBlock();
-
-    final IbftExtraData prevExtraData = IbftExtraData.decode(block.getHeader().getExtraData());
-    final IbftExtraData extraDataToPublish =
-        new IbftExtraData(
-            prevExtraData.getVanityData(),
-            prevExtraData.getSeals(),
-            prevExtraData.getVote(),
+  private Proposal createProposalAroundBlock(final Block block) {
+    final Block blockToPublish =
+        IbftBlockInterface.replaceRoundInBlock(
+            block,
             getRoundIdentifier().getRoundNumber(),
-            prevExtraData.getValidators());
-
-    final BlockHeaderBuilder headerBuilder = BlockHeaderBuilder.fromHeader(block.getHeader());
-    headerBuilder
-        .extraData(extraDataToPublish.encode())
-        .blockHashFunction(
-            blockHeader ->
-                IbftBlockHashing.calculateDataHashForCommittedSeal(
-                    blockHeader, extraDataToPublish));
-    final BlockHeader newHeader = headerBuilder.buildBlockHeader();
-    final Block newBlock = new Block(newHeader, block.getBody());
-    LOG.debug(
-        "Created proposal from prepared certificate blockHeader={} extraData={}",
-        block.getHeader(),
-        extraDataToPublish);
-    return messageFactory.createSignedProposalPayload(getRoundIdentifier(), newBlock);
+            IbftBlockHashing::calculateDataHashForCommittedSeal);
+    return messageFactory.createSignedProposalPayload(getRoundIdentifier(), blockToPublish);
   }
 
   public void handleProposalMessage(final Proposal msg) {

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

@@ -0,0 +1,62 @@
+/*
+ * 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.statemachine;
+
+import tech.pegasys.pantheon.consensus.ibft.IbftHelpers;
+import tech.pegasys.pantheon.consensus.ibft.messagewrappers.RoundChange;
+import tech.pegasys.pantheon.consensus.ibft.payload.PreparedCertificate;
+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.ethereum.core.Block;
+
+import java.util.Collection;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class RoundChangeArtefacts {
+
+  private final Optional<Block> block;
+  private final Collection<SignedData<RoundChangePayload>> roundChangePayloads;
+
+  public RoundChangeArtefacts(
+      final Optional<Block> block,
+      final Collection<SignedData<RoundChangePayload>> roundChangePayloads) {
+    this.block = block;
+    this.roundChangePayloads = roundChangePayloads;
+  }
+
+  public Optional<Block> getBlock() {
+    return block;
+  }
+
+  public RoundChangeCertificate getRoundChangeCertificate() {
+    return new RoundChangeCertificate(roundChangePayloads);
+  }
+
+  public static RoundChangeArtefacts create(final Collection<RoundChange> roundChanges) {
+
+    final Collection<SignedData<RoundChangePayload>> payloads =
+        roundChanges
+            .stream()
+            .map(roundChange -> roundChange.getSignedPayload())
+            .collect(Collectors.toList());
+
+    final Optional<PreparedCertificate> latestPreparedCertificate =
+        IbftHelpers.findLatestPreparedCertificate(payloads);
+
+    return new RoundChangeArtefacts(
+        latestPreparedCertificate.map(cert -> cert.getProposalPayload().getPayload().getBlock()),
+        payloads);
+  }
+}

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

@@ -14,13 +14,12 @@
 
 import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.RoundChange;
-import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangeCertificate;
 import tech.pegasys.pantheon.consensus.ibft.validation.RoundChangeMessageValidator;
 import tech.pegasys.pantheon.ethereum.core.Address;
 
+import java.util.Collection;
 import java.util.Map;
 import java.util.Optional;
-import java.util.stream.Collectors;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Maps;
@@ -61,15 +60,10 @@ public boolean roundChangeReady() {
       return receivedMessages.size() >= quorum && !actioned;
     }
 
-    public RoundChangeCertificate createRoundChangeCertificate() {
+    public Collection<RoundChange> createRoundChangeCertificate() {
       if (roundChangeReady()) {
         actioned = true;
-        return new RoundChangeCertificate(
-            receivedMessages
-                .values()
-                .stream()
-                .map(RoundChange::getSignedPayload)
-                .collect(Collectors.toList()));
+        return receivedMessages.values();
       } else {
         throw new IllegalStateException("Unable to create RoundChangeCertificate at this time.");
       }
@@ -97,7 +91,7 @@ public RoundChangeManager(
    * @return Empty if the round change threshold hasn't been hit, otherwise a round change
    *     certificate
    */
-  public Optional<RoundChangeCertificate> appendRoundChangeMessage(final RoundChange msg) {
+  public Optional<Collection<RoundChange>> appendRoundChangeMessage(final RoundChange msg) {
 
     if (!isMessageValid(msg)) {
       LOG.info("RoundChange message was invalid.");

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

@@ -218,8 +218,7 @@ public void onRoundChangeReceptionRoundChangeManagerIsInvokedAndNewRoundStarted(
     final RoundChange roundChange =
         messageFactory.createSignedRoundChangePayload(futureRoundIdentifier, Optional.empty());
     when(roundChangeManager.appendRoundChangeMessage(any()))
-        .thenReturn(
-            Optional.of(new RoundChangeCertificate(singletonList(roundChange.getSignedPayload()))));
+        .thenReturn(Optional.of(singletonList(roundChange)));
     when(finalState.isLocalNodeProposerForRound(any())).thenReturn(false);
 
     final IbftBlockHeightManager manager =
@@ -266,7 +265,7 @@ public void whenSufficientRoundChangesAreReceivedANewRoundMessageIsTransmitted()
         new RoundChangeCertificate(singletonList(roundChange.getSignedPayload()));
 
     when(roundChangeManager.appendRoundChangeMessage(any()))
-        .thenReturn(Optional.of(roundChangCert));
+        .thenReturn(Optional.of(singletonList(roundChange)));
     when(finalState.isLocalNodeProposerForRound(any())).thenReturn(true);
 
     final IbftBlockHeightManager manager =