Ibft2: Replace NewRound with extended Proposal

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

@@ -25,6 +25,7 @@
 import tech.pegasys.pantheon.crypto.SECP256K1.Signature;
 import tech.pegasys.pantheon.ethereum.core.Block;
 
+import java.util.Optional;
 import java.util.stream.Collectors;
 
 public class IntegrationTestHelpers {
@@ -49,7 +50,10 @@ public static PreparedRoundArtifacts createValidPreparedRoundArtifacts(
     final RoundSpecificPeers peers = context.roundSpecificPeers(preparedRound);
 
     return new PreparedRoundArtifacts(
-        peers.getProposer().getMessageFactory().createProposal(preparedRound, block),
+        peers
+            .getProposer()
+            .getMessageFactory()
+            .createProposal(preparedRound, block, Optional.empty()),
         peers.createSignedPreparePayloadOfNonProposing(preparedRound, block.getHash()).stream()
             .map(Prepare::new)
             .collect(Collectors.toList()));

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

@@ -19,7 +19,6 @@
 import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
 import tech.pegasys.pantheon.consensus.ibft.messagedata.CommitMessageData;
 import tech.pegasys.pantheon.consensus.ibft.messagedata.IbftV2;
-import tech.pegasys.pantheon.consensus.ibft.messagedata.NewRoundMessageData;
 import tech.pegasys.pantheon.consensus.ibft.messagedata.PrepareMessageData;
 import tech.pegasys.pantheon.consensus.ibft.messagedata.ProposalMessageData;
 import tech.pegasys.pantheon.consensus.ibft.messagedata.RoundChangeMessageData;
@@ -197,9 +196,6 @@ private void verifyMessage(final MessageData actual, final IbftMessage<?> expect
       case IbftV2.COMMIT:
         actualSignedPayload = CommitMessageData.fromMessageData(actual).decode();
         break;
-      case IbftV2.NEW_ROUND:
-        actualSignedPayload = NewRoundMessageData.fromMessageData(actual).decode();
-        break;
       case IbftV2.ROUND_CHANGE:
         actualSignedPayload = RoundChangeMessageData.fromMessageData(actual).decode();
         break;

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

@@ -16,19 +16,15 @@
 import tech.pegasys.pantheon.consensus.ibft.EventMultiplexer;
 import tech.pegasys.pantheon.consensus.ibft.ibftevent.IbftEvents;
 import tech.pegasys.pantheon.consensus.ibft.messagedata.CommitMessageData;
-import tech.pegasys.pantheon.consensus.ibft.messagedata.NewRoundMessageData;
 import tech.pegasys.pantheon.consensus.ibft.messagedata.PrepareMessageData;
 import tech.pegasys.pantheon.consensus.ibft.messagedata.ProposalMessageData;
 import tech.pegasys.pantheon.consensus.ibft.messagedata.RoundChangeMessageData;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Commit;
-import tech.pegasys.pantheon.consensus.ibft.messagewrappers.NewRound;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Prepare;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Proposal;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.RoundChange;
 import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory;
 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.consensus.ibft.statemachine.PreparedRoundArtifacts;
 import tech.pegasys.pantheon.crypto.SECP256K1;
 import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair;
@@ -79,7 +75,7 @@ public KeyPair getNodeKeys() {
   }
 
   public Proposal injectProposal(final ConsensusRoundIdentifier rId, final Block block) {
-    final Proposal payload = messageFactory.createProposal(rId, block);
+    final Proposal payload = messageFactory.createProposal(rId, block, Optional.empty());
 
     injectMessage(ProposalMessageData.create(payload));
     return payload;
@@ -108,26 +104,15 @@ public Commit injectCommit(
     return payload;
   }
 
-  public NewRound injectNewRound(
+  public Proposal injectProposalForFutureRound(
       final ConsensusRoundIdentifier rId,
       final RoundChangeCertificate roundChangeCertificate,
-      final Proposal proposal) {
-
-    final NewRound payload =
-        messageFactory.createNewRound(
-            rId, roundChangeCertificate, proposal.getSignedPayload(), proposal.getBlock());
-    injectMessage(NewRoundMessageData.create(payload));
-    return payload;
-  }
-
-  public NewRound injectEmptyNewRound(
-      final ConsensusRoundIdentifier targetRoundId,
-      final List<SignedData<RoundChangePayload>> roundChangePayloads,
       final Block blockToPropose) {
 
-    final Proposal proposal = messageFactory.createProposal(targetRoundId, blockToPropose);
-
-    return injectNewRound(targetRoundId, new RoundChangeCertificate(roundChangePayloads), proposal);
+    final Proposal payload =
+        messageFactory.createProposal(rId, blockToPropose, Optional.of(roundChangeCertificate));
+    injectMessage(ProposalMessageData.create(payload));
+    return payload;
   }
 
   public RoundChange injectRoundChange(

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

@@ -19,6 +19,7 @@
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Commit;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Prepare;
 import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory;
+import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangeCertificate;
 import tech.pegasys.pantheon.consensus.ibft.support.RoundSpecificPeers;
 import tech.pegasys.pantheon.consensus.ibft.support.TestContext;
 import tech.pegasys.pantheon.consensus.ibft.support.TestContextBuilder;
@@ -93,8 +94,10 @@ public void messagesForFutureRoundAreNotActionedUntilRoundIsActive() {
     // and updates blockchain
     futurePeers
         .getProposer()
-        .injectEmptyNewRound(
-            futureRoundId, futurePeers.createSignedRoundChangePayload(futureRoundId), futureBlock);
+        .injectProposalForFutureRound(
+            futureRoundId,
+            new RoundChangeCertificate(futurePeers.createSignedRoundChangePayload(futureRoundId)),
+            futureBlock);
 
     final Prepare expectedPrepare =
         localNodeMessageFactory.createPrepare(futureRoundId, futureBlock.getHash());
@@ -136,8 +139,10 @@ public void priorRoundsCannotBeCompletedAfterReceptionOfNewRound() {
 
     futurePeers
         .getProposer()
-        .injectEmptyNewRound(
-            futureRoundId, futurePeers.createSignedRoundChangePayload(futureRoundId), futureBlock);
+        .injectProposalForFutureRound(
+            futureRoundId,
+            new RoundChangeCertificate(futurePeers.createSignedRoundChangePayload(futureRoundId)),
+            futureBlock);
 
     final Prepare expectedFuturePrepare =
         localNodeMessageFactory.createPrepare(futureRoundId, futureBlock.getHash());

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

@@ -20,7 +20,6 @@
 import tech.pegasys.pantheon.consensus.ibft.ibftevent.NewChainHead;
 import tech.pegasys.pantheon.consensus.ibft.messagedata.ProposalMessageData;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Commit;
-import tech.pegasys.pantheon.consensus.ibft.messagewrappers.NewRound;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Prepare;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Proposal;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.RoundChange;
@@ -93,8 +92,10 @@ public void gossipMessagesToPeers() {
     final RoundChange roundChange = msgFactory.createRoundChange(roundId, Optional.empty());
     final RoundChangeCertificate roundChangeCert =
         new RoundChangeCertificate(singleton(roundChange.getSignedPayload()));
-    final NewRound newRound = sender.injectNewRound(roundId, roundChangeCert, proposal);
-    peers.verifyMessagesReceivedNonPropsing(newRound);
+
+    final Proposal nextRoundProposal =
+        sender.injectProposalForFutureRound(roundId, roundChangeCert, proposal.getBlock());
+    peers.verifyMessagesReceivedNonPropsing(nextRoundProposal);
     peers.verifyNoMessagesReceivedProposer();
 
     sender.injectRoundChange(roundId, Optional.empty());
@@ -118,7 +119,8 @@ public void onlyGossipOnce() {
   public void messageWithUnknownValidatorIsNotGossiped() {
     final KeyPair unknownKeyPair = KeyPair.generate();
     final MessageFactory unknownMsgFactory = new MessageFactory(unknownKeyPair);
-    final Proposal unknownProposal = unknownMsgFactory.createProposal(roundId, block);
+    final Proposal unknownProposal =
+        unknownMsgFactory.createProposal(roundId, block, Optional.empty());
 
     sender.injectMessage(ProposalMessageData.create(unknownProposal));
     peers.verifyNoMessagesReceived();
@@ -128,7 +130,8 @@ public void messageWithUnknownValidatorIsNotGossiped() {
   public void messageIsNotGossipedToSenderOrCreator() {
     final ValidatorPeer msgCreator = peers.getFirstNonProposer();
     final MessageFactory peerMsgFactory = msgCreator.getMessageFactory();
-    final Proposal proposalFromPeer = peerMsgFactory.createProposal(roundId, block);
+    final Proposal proposalFromPeer =
+        peerMsgFactory.createProposal(roundId, block, Optional.empty());
 
     sender.injectMessage(ProposalMessageData.create(proposalFromPeer));
 
@@ -139,7 +142,7 @@ public void messageIsNotGossipedToSenderOrCreator() {
   @Test
   public void futureMessageIsNotGossipedImmediately() {
     ConsensusRoundIdentifier futureRoundId = new ConsensusRoundIdentifier(2, 0);
-    msgFactory.createProposal(futureRoundId, block);
+    msgFactory.createProposal(futureRoundId, block, Optional.empty());
 
     sender.injectProposal(futureRoundId, block);
     peers.verifyNoMessagesReceived();

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

@@ -28,6 +28,7 @@
 import java.time.Clock;
 import java.time.Instant;
 import java.time.ZoneId;
+import java.util.Optional;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -63,7 +64,8 @@ public class LocalNodeIsProposerTest {
   @Before
   public void setup() {
     expectedProposedBlock = context.createBlockForProposalFromChainHead(0, blockTimeStamp);
-    expectedTxProposal = localNodeMessageFactory.createProposal(roundId, expectedProposedBlock);
+    expectedTxProposal =
+        localNodeMessageFactory.createProposal(roundId, expectedProposedBlock, Optional.empty());
 
     expectedTxCommit =
         new Commit(

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

@@ -17,7 +17,6 @@
 import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Commit;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Prepare;
-import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Proposal;
 import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory;
 import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangeCertificate;
 import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangePayload;
@@ -35,8 +34,11 @@
 import org.junit.Before;
 import org.junit.Test;
 
-/** Ensure the Ibft component responds appropriately when a NewRound message is received. */
-public class ReceivedNewRoundTest {
+/**
+ * Ensure the Ibft component responds appropriately when a future round Proposal message is
+ * received.
+ */
+public class ReceivedFutureProposalTest {
 
   private final int NETWORK_SIZE = 5;
 
@@ -57,7 +59,7 @@ public void setup() {
   }
 
   @Test
-  public void newRoundMessageWithEmptyPrepareCertificatesOfferNewBlock() {
+  public void proposalWithEmptyPrepareCertificatesOfferNewBlock() {
     final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
     final Block blockToPropose =
         context.createBlockForProposalFromChainHead(nextRoundId.getRoundNumber(), 15);
@@ -68,10 +70,8 @@ public void newRoundMessageWithEmptyPrepareCertificatesOfferNewBlock() {
 
     final ValidatorPeer nextProposer = context.roundSpecificPeers(nextRoundId).getProposer();
 
-    nextProposer.injectNewRound(
-        targetRound,
-        new RoundChangeCertificate(roundChanges),
-        nextProposer.getMessageFactory().createProposal(targetRound, blockToPropose));
+    nextProposer.injectProposalForFutureRound(
+        targetRound, new RoundChangeCertificate(roundChanges), blockToPropose);
 
     final Prepare expectedPrepare =
         localNodeMessageFactory.createPrepare(targetRound, blockToPropose.getHash());
@@ -80,7 +80,7 @@ public void newRoundMessageWithEmptyPrepareCertificatesOfferNewBlock() {
   }
 
   @Test
-  public void newRoundMessageFromIllegalSenderIsDiscardedAndNoPrepareForNewRoundIsSent() {
+  public void proposalFromIllegalSenderIsDiscardedAndNoPrepareForNewRoundIsSent() {
     final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
     final Block blockToPropose =
         context.createBlockForProposalFromChainHead(nextRoundId.getRoundNumber(), 15);
@@ -91,16 +91,14 @@ public void newRoundMessageFromIllegalSenderIsDiscardedAndNoPrepareForNewRoundIs
     final ValidatorPeer illegalProposer =
         context.roundSpecificPeers(nextRoundId).getNonProposing(0);
 
-    illegalProposer.injectNewRound(
-        nextRoundId,
-        new RoundChangeCertificate(roundChanges),
-        illegalProposer.getMessageFactory().createProposal(nextRoundId, blockToPropose));
+    illegalProposer.injectProposalForFutureRound(
+        nextRoundId, new RoundChangeCertificate(roundChanges), blockToPropose);
 
     peers.verifyNoMessagesReceived();
   }
 
   @Test
-  public void newRoundWithPrepareCertificateResultsInNewRoundStartingWithExpectedBlock() {
+  public void proposalWithPrepareCertificateResultsInNewRoundStartingWithExpectedBlock() {
     final Block initialBlock = context.createBlockForProposalFromChainHead(0, 15);
     final Block reproposedBlock = context.createBlockForProposalFromChainHead(1, 15);
     final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
@@ -113,18 +111,16 @@ public void newRoundWithPrepareCertificateResultsInNewRoundStartingWithExpectedB
 
     final ValidatorPeer nextProposer = context.roundSpecificPeers(nextRoundId).getProposer();
 
-    nextProposer.injectNewRound(
-        nextRoundId,
-        new RoundChangeCertificate(roundChanges),
-        peers.getNonProposing(0).getMessageFactory().createProposal(nextRoundId, reproposedBlock));
+    nextProposer.injectProposalForFutureRound(
+        nextRoundId, new RoundChangeCertificate(roundChanges), reproposedBlock);
 
     peers.verifyMessagesReceived(
         localNodeMessageFactory.createPrepare(nextRoundId, reproposedBlock.getHash()));
   }
 
   @Test
-  public void newRoundMessageForPriorRoundIsNotActioned() {
-    // first move to a future round, then inject a newRound for a prior round, local node
+  public void proposalMessageForPriorRoundIsNotActioned() {
+    // first move to a future round, then inject a proposal for a prior round, local node
     // should send no messages.
     final ConsensusRoundIdentifier futureRound = new ConsensusRoundIdentifier(1, 2);
     peers.roundChange(futureRound);
@@ -136,19 +132,16 @@ public void newRoundMessageForPriorRoundIsNotActioned() {
     final ValidatorPeer interimRoundProposer =
         context.roundSpecificPeers(interimRound).getProposer();
 
-    final Proposal proposal =
-        interimRoundProposer
-            .getMessageFactory()
-            .createProposal(interimRound, context.createBlockForProposalFromChainHead(1, 30));
-
-    interimRoundProposer.injectNewRound(
-        interimRound, new RoundChangeCertificate(roundChangePayloads), proposal);
+    interimRoundProposer.injectProposalForFutureRound(
+        interimRound,
+        new RoundChangeCertificate(roundChangePayloads),
+        context.createBlockForProposalFromChainHead(1, 30));
 
     peers.verifyNoMessagesReceived();
   }
 
   @Test
-  public void receiveRoundStateIsNotLostIfASecondNewRoundMessageIsReceivedForCurrentRound() {
+  public void receiveRoundStateIsNotLostIfASecondProposalMessageIsReceivedForCurrentRound() {
     final Block initialBlock = context.createBlockForProposalFromChainHead(0, 15);
     final Block reproposedBlock = context.createBlockForProposalFromChainHead(1, 15);
     final ConsensusRoundIdentifier nextRoundId = new ConsensusRoundIdentifier(1, 1);
@@ -162,22 +155,18 @@ public void receiveRoundStateIsNotLostIfASecondNewRoundMessageIsReceivedForCurre
     final RoundSpecificPeers nextRoles = context.roundSpecificPeers(nextRoundId);
     final ValidatorPeer nextProposer = nextRoles.getProposer();
 
-    nextProposer.injectNewRound(
-        nextRoundId,
-        new RoundChangeCertificate(roundChanges),
-        peers.getNonProposing(0).getMessageFactory().createProposal(nextRoundId, reproposedBlock));
+    nextProposer.injectProposalForFutureRound(
+        nextRoundId, new RoundChangeCertificate(roundChanges), reproposedBlock);
 
     peers.verifyMessagesReceived(
         localNodeMessageFactory.createPrepare(nextRoundId, reproposedBlock.getHash()));
 
-    // Inject a prepare, then re-inject the newRound - then ensure only a single prepare is enough
+    // Inject a prepare, then re-inject the proposal - then ensure only a single prepare is enough
     // to trigger a Commit transmission from the local node
     nextRoles.getNonProposing(0).injectPrepare(nextRoundId, reproposedBlock.getHash());
 
-    nextProposer.injectNewRound(
-        nextRoundId,
-        new RoundChangeCertificate(roundChanges),
-        peers.getNonProposing(0).getMessageFactory().createProposal(nextRoundId, reproposedBlock));
+    nextProposer.injectProposalForFutureRound(
+        nextRoundId, new RoundChangeCertificate(roundChanges), reproposedBlock);
 
     peers.verifyNoMessagesReceived();