Update RoundChangeManager to use flattened message (PegaSysEng#742)

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

@@ -65,8 +65,7 @@ public void send(final Message message) {
         decodedMessage = RoundChangeMessageData.fromMessageData(messageData).decode();
         break;
       case IbftV2.NEW_ROUND:
-        decodedMessage =
-            NewRoundMessageData.fromMessageData(messageData).decode().getSignedPayload();
+        decodedMessage = NewRoundMessageData.fromMessageData(messageData).decode();
         break;
       default:
         throw new IllegalArgumentException(

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

@@ -204,7 +204,7 @@ public void handleRoundChangePayload(final RoundChange message) {
     }
 
     final Optional<RoundChangeCertificate> result =
-        roundChangeManager.appendRoundChangeMessage(message.getSignedPayload());
+        roundChangeManager.appendRoundChangeMessage(message);
     if (result.isPresent()) {
       if (messageAge == FUTURE_ROUND) {
         startNewRound(targetRound.getRoundNumber());

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

@@ -13,14 +13,14 @@
 package tech.pegasys.pantheon.consensus.ibft.statemachine;
 
 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.payload.RoundChangePayload;
-import tech.pegasys.pantheon.consensus.ibft.payload.SignedData;
 import tech.pegasys.pantheon.consensus.ibft.validation.RoundChangeMessageValidator;
 import tech.pegasys.pantheon.ethereum.core.Address;
 
 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;
@@ -43,16 +43,15 @@ public static class RoundChangeStatus {
     private final long quorum;
 
     // Store only 1 round change per round per validator
-    @VisibleForTesting
-    final Map<Address, SignedData<RoundChangePayload>> receivedMessages = Maps.newLinkedHashMap();
+    @VisibleForTesting final Map<Address, RoundChange> receivedMessages = Maps.newLinkedHashMap();
 
     private boolean actioned = false;
 
     public RoundChangeStatus(final long quorum) {
       this.quorum = quorum;
     }
 
-    public void addMessage(final SignedData<RoundChangePayload> msg) {
+    public void addMessage(final RoundChange msg) {
       if (!actioned) {
         receivedMessages.putIfAbsent(msg.getAuthor(), msg);
       }
@@ -65,7 +64,12 @@ public boolean roundChangeReady() {
     public RoundChangeCertificate createRoundChangeCertificate() {
       if (roundChangeReady()) {
         actioned = true;
-        return new RoundChangeCertificate(receivedMessages.values());
+        return new RoundChangeCertificate(
+            receivedMessages
+                .values()
+                .stream()
+                .map(RoundChange::getSignedPayload)
+                .collect(Collectors.toList()));
       } else {
         throw new IllegalStateException("Unable to create RoundChangeCertificate at this time.");
       }
@@ -93,8 +97,7 @@ public RoundChangeManager(
    * @return Empty if the round change threshold hasn't been hit, otherwise a round change
    *     certificate
    */
-  public Optional<RoundChangeCertificate> appendRoundChangeMessage(
-      final SignedData<RoundChangePayload> msg) {
+  public Optional<RoundChangeCertificate> appendRoundChangeMessage(final RoundChange msg) {
 
     if (!isMessageValid(msg)) {
       LOG.info("RoundChange message was invalid.");
@@ -110,12 +113,12 @@ public Optional<RoundChangeCertificate> appendRoundChangeMessage(
     return Optional.empty();
   }
 
-  private boolean isMessageValid(final SignedData<RoundChangePayload> msg) {
-    return roundChangeMessageValidator.validateMessage(msg);
+  private boolean isMessageValid(final RoundChange msg) {
+    return roundChangeMessageValidator.validateMessage(msg.getSignedPayload());
   }
 
-  private RoundChangeStatus storeRoundChangeMessage(final SignedData<RoundChangePayload> msg) {
-    final ConsensusRoundIdentifier msgTargetRound = msg.getPayload().getRoundIdentifier();
+  private RoundChangeStatus storeRoundChangeMessage(final RoundChange msg) {
+    final ConsensusRoundIdentifier msgTargetRound = msg.getRoundIdentifier();
 
     final RoundChangeStatus roundChangeStatus =
         roundChangeCache.computeIfAbsent(msgTargetRound, ignored -> new RoundChangeStatus(quorum));

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

@@ -237,7 +237,7 @@ public void onRoundChangeReceptionRoundChangeManagerIsInvokedAndNewRoundStarted(
 
     manager.handleRoundChangePayload(roundChange);
 
-    verify(roundChangeManager, times(1)).appendRoundChangeMessage(roundChange.getSignedPayload());
+    verify(roundChangeManager, times(1)).appendRoundChangeMessage(roundChange);
     verify(roundFactory, times(1))
         .createNewRound(any(), eq(futureRoundIdentifier.getRoundNumber()));
   }

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

@@ -21,11 +21,11 @@
 import tech.pegasys.pantheon.consensus.ibft.IbftContext;
 import tech.pegasys.pantheon.consensus.ibft.IbftHelpers;
 import tech.pegasys.pantheon.consensus.ibft.TestHelpers;
+import tech.pegasys.pantheon.consensus.ibft.messagewrappers.RoundChange;
 import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory;
 import tech.pegasys.pantheon.consensus.ibft.payload.PreparePayload;
 import tech.pegasys.pantheon.consensus.ibft.payload.PreparedCertificate;
 import tech.pegasys.pantheon.consensus.ibft.payload.ProposalPayload;
-import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangePayload;
 import tech.pegasys.pantheon.consensus.ibft.payload.SignedData;
 import tech.pegasys.pantheon.consensus.ibft.validation.MessageValidator;
 import tech.pegasys.pantheon.consensus.ibft.validation.RoundChangeMessageValidator;
@@ -126,13 +126,13 @@ public void setup() {
     manager = new RoundChangeManager(2, roundChangeMessageValidator);
   }
 
-  private SignedData<RoundChangePayload> makeRoundChangeMessage(
+  private RoundChange makeRoundChangeMessage(
       final KeyPair key, final ConsensusRoundIdentifier round) {
     MessageFactory messageFactory = new MessageFactory(key);
-    return messageFactory.createSignedRoundChangePayload(round, Optional.empty());
+    return new RoundChange(messageFactory.createSignedRoundChangePayload(round, Optional.empty()));
   }
 
-  private SignedData<RoundChangePayload> makeRoundChangeMessageWithPreparedCert(
+  private RoundChange makeRoundChangeMessageWithPreparedCert(
       final KeyPair key,
       final ConsensusRoundIdentifier round,
       final List<KeyPair> prepareProviders) {
@@ -158,48 +158,44 @@ private SignedData<RoundChangePayload> makeRoundChangeMessageWithPreparedCert(
 
     final PreparedCertificate cert = new PreparedCertificate(proposal, preparePayloads);
 
-    return messageFactory.createSignedRoundChangePayload(round, Optional.of(cert));
+    return new RoundChange(messageFactory.createSignedRoundChangePayload(round, Optional.of(cert)));
   }
 
   @Test
   public void rejectsInvalidRoundChangeMessage() {
-    SignedData<RoundChangePayload> roundChangeData = makeRoundChangeMessage(nonValidatorKey, ri1);
+    final RoundChange roundChangeData = makeRoundChangeMessage(nonValidatorKey, ri1);
     assertThat(manager.appendRoundChangeMessage(roundChangeData)).isEmpty();
     assertThat(manager.roundChangeCache.get(ri1)).isNull();
   }
 
   @Test
   public void acceptsValidRoundChangeMessage() {
-    SignedData<RoundChangePayload> roundChangeData = makeRoundChangeMessage(proposerKey, ri2);
+    final RoundChange roundChangeData = makeRoundChangeMessage(proposerKey, ri2);
     assertThat(manager.appendRoundChangeMessage(roundChangeData)).isEmpty();
     assertThat(manager.roundChangeCache.get(ri2).receivedMessages.size()).isEqualTo(1);
   }
 
   @Test
   public void doesntAcceptDuplicateValidRoundChangeMessage() {
-    SignedData<RoundChangePayload> roundChangeData = makeRoundChangeMessage(proposerKey, ri2);
+    final RoundChange roundChangeData = makeRoundChangeMessage(proposerKey, ri2);
     assertThat(manager.appendRoundChangeMessage(roundChangeData)).isEmpty();
     assertThat(manager.appendRoundChangeMessage(roundChangeData)).isEmpty();
     assertThat(manager.roundChangeCache.get(ri2).receivedMessages.size()).isEqualTo(1);
   }
 
   @Test
   public void becomesReadyAtThreshold() {
-    SignedData<RoundChangePayload> roundChangeDataProposer =
-        makeRoundChangeMessage(proposerKey, ri2);
-    SignedData<RoundChangePayload> roundChangeDataValidator1 =
-        makeRoundChangeMessage(validator1Key, ri2);
+    final RoundChange roundChangeDataProposer = makeRoundChangeMessage(proposerKey, ri2);
+    final RoundChange roundChangeDataValidator1 = makeRoundChangeMessage(validator1Key, ri2);
     assertThat(manager.appendRoundChangeMessage(roundChangeDataProposer))
         .isEqualTo(Optional.empty());
     assertThat(manager.appendRoundChangeMessage(roundChangeDataValidator1).isPresent()).isTrue();
   }
 
   @Test
   public void doesntReachReadyWhenSuppliedWithDifferentRounds() {
-    SignedData<RoundChangePayload> roundChangeDataProposer =
-        makeRoundChangeMessage(proposerKey, ri2);
-    SignedData<RoundChangePayload> roundChangeDataValidator1 =
-        makeRoundChangeMessage(validator1Key, ri3);
+    final RoundChange roundChangeDataProposer = makeRoundChangeMessage(proposerKey, ri2);
+    final RoundChange roundChangeDataValidator1 = makeRoundChangeMessage(validator1Key, ri3);
     assertThat(manager.appendRoundChangeMessage(roundChangeDataProposer))
         .isEqualTo(Optional.empty());
     assertThat(manager.appendRoundChangeMessage(roundChangeDataValidator1))
@@ -210,12 +206,9 @@ public void doesntReachReadyWhenSuppliedWithDifferentRounds() {
 
   @Test
   public void discardsRoundPreviousToThatRequested() {
-    SignedData<RoundChangePayload> roundChangeDataProposer =
-        makeRoundChangeMessage(proposerKey, ri1);
-    SignedData<RoundChangePayload> roundChangeDataValidator1 =
-        makeRoundChangeMessage(validator1Key, ri2);
-    SignedData<RoundChangePayload> roundChangeDataValidator2 =
-        makeRoundChangeMessage(validator2Key, ri3);
+    final RoundChange roundChangeDataProposer = makeRoundChangeMessage(proposerKey, ri1);
+    final RoundChange roundChangeDataValidator1 = makeRoundChangeMessage(validator1Key, ri2);
+    final RoundChange roundChangeDataValidator2 = makeRoundChangeMessage(validator2Key, ri3);
     assertThat(manager.appendRoundChangeMessage(roundChangeDataProposer))
         .isEqualTo(Optional.empty());
     assertThat(manager.appendRoundChangeMessage(roundChangeDataValidator1))
@@ -230,12 +223,9 @@ public void discardsRoundPreviousToThatRequested() {
 
   @Test
   public void stopsAcceptingMessagesAfterReady() {
-    SignedData<RoundChangePayload> roundChangeDataProposer =
-        makeRoundChangeMessage(proposerKey, ri2);
-    SignedData<RoundChangePayload> roundChangeDataValidator1 =
-        makeRoundChangeMessage(validator1Key, ri2);
-    SignedData<RoundChangePayload> roundChangeDataValidator2 =
-        makeRoundChangeMessage(validator2Key, ri2);
+    final RoundChange roundChangeDataProposer = makeRoundChangeMessage(proposerKey, ri2);
+    final RoundChange roundChangeDataValidator1 = makeRoundChangeMessage(validator1Key, ri2);
+    final RoundChange roundChangeDataValidator2 = makeRoundChangeMessage(validator2Key, ri2);
     assertThat(manager.appendRoundChangeMessage(roundChangeDataProposer))
         .isEqualTo(Optional.empty());
     assertThat(manager.appendRoundChangeMessage(roundChangeDataValidator1).isPresent()).isTrue();
@@ -252,7 +242,7 @@ public void roundChangeMessagesWithPreparedCertificateMustHaveSufficientPrepareM
     // There are 3 validators, therefore, should only need 2 prepare message to be acceptable.
 
     // These tests are run at ri2, such that validators can be found for past round at ri1.
-    SignedData<RoundChangePayload> roundChangeData =
+    RoundChange roundChangeData =
         makeRoundChangeMessageWithPreparedCert(proposerKey, ri2, Collections.emptyList());
     assertThat(manager.appendRoundChangeMessage(roundChangeData)).isEmpty();
     assertThat(manager.roundChangeCache.get(ri2)).isNull();