From 79e748393b031df52ddb35c7ca228c0ba16ceb32 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 20 Dec 2018 10:03:23 +1000 Subject: [PATCH 01/33] [NC-1909] renamed message types to message data to match their heirachy --- .../pantheon/consensus/ibft/IbftMessages.java | 20 +++++++++---------- ...sage.java => AbstractIbftMessageData.java} | 18 ++++++++--------- ...mitMessage.java => CommitMessageData.java} | 12 +++++------ ...dMessage.java => NewRoundMessageData.java} | 12 +++++------ ...reMessage.java => PrepareMessageData.java} | 12 +++++------ ...lMessage.java => ProposalMessageData.java} | 12 +++++------ ...ssage.java => RoundChangeMessageData.java} | 12 +++++------ .../ibft/statemachine/IbftController.java | 20 +++++++++---------- .../statemachine/IbftMessageTransmitter.java | 20 +++++++++---------- .../ibft/ibftmessage/CommitMessageTest.java | 12 +++++------ .../ibft/ibftmessage/NewRoundMessageTest.java | 12 +++++------ .../ibftmessage/PrePrepareMessageTest.java | 12 +++++------ .../ibft/ibftmessage/PrepareMessageTest.java | 12 +++++------ .../ibftmessage/RoundChangeMessageTest.java | 12 +++++------ .../ibft/statemachine/IbftControllerTest.java | 20 +++++++++---------- 15 files changed, 109 insertions(+), 109 deletions(-) rename consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/{AbstractIbftMessage.java => AbstractIbftMessageData.java} (70%) rename consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/{CommitMessage.java => CommitMessageData.java} (73%) rename consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/{NewRoundMessage.java => NewRoundMessageData.java} (72%) rename consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/{PrepareMessage.java => PrepareMessageData.java} (72%) rename consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/{ProposalMessage.java => ProposalMessageData.java} (72%) rename consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/{RoundChangeMessage.java => RoundChangeMessageData.java} (71%) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftMessages.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftMessages.java index 3eebbb8635..a6c4954ddc 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftMessages.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftMessages.java @@ -12,12 +12,12 @@ */ package tech.pegasys.pantheon.consensus.ibft; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessage; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.IbftV2; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessage; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; import tech.pegasys.pantheon.ethereum.p2p.api.Message; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; @@ -29,19 +29,19 @@ public static SignedData fromMessage(final Message message) { switch (messageData.getCode()) { case IbftV2.PROPOSAL: - return ProposalMessage.fromMessage(messageData).decode(); + return ProposalMessageData.fromMessageData(messageData).decode(); case IbftV2.PREPARE: - return PrepareMessage.fromMessage(messageData).decode(); + return PrepareMessageData.fromMessageData(messageData).decode(); case IbftV2.COMMIT: - return CommitMessage.fromMessage(messageData).decode(); + return CommitMessageData.fromMessageData(messageData).decode(); case IbftV2.ROUND_CHANGE: - return RoundChangeMessage.fromMessage(messageData).decode(); + return RoundChangeMessageData.fromMessageData(messageData).decode(); case IbftV2.NEW_ROUND: - return NewRoundMessage.fromMessage(messageData).decode(); + return NewRoundMessageData.fromMessageData(messageData).decode(); default: throw new IllegalArgumentException( diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessage.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java similarity index 70% rename from consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessage.java rename to consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java index 212dc85bcc..0970b40bd9 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessage.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java @@ -19,29 +19,29 @@ import java.util.function.Function; -public abstract class AbstractIbftMessage extends AbstractMessageData { - protected AbstractIbftMessage(final BytesValue data) { +public abstract class AbstractIbftMessageData extends AbstractMessageData { + protected AbstractIbftMessageData(final BytesValue data) { super(data); } public abstract SignedData decode(); - protected static T fromMessage( - final MessageData message, + protected static T fromMessageData( + final MessageData messageData, final int messageCode, final Class clazz, final Function constructor) { - if (clazz.isInstance(message)) { + if (clazz.isInstance(messageData)) { @SuppressWarnings("unchecked") - T castMessage = (T) message; + T castMessage = (T) messageData; return castMessage; } - final int code = message.getCode(); + final int code = messageData.getCode(); if (code != messageCode) { throw new IllegalArgumentException( - String.format("Message has code %d and thus is not a %s", code, clazz.getSimpleName())); + String.format("MessageData has code %d and thus is not a %s", code, clazz.getSimpleName())); } - return constructor.apply(message.getData()); + return constructor.apply(messageData.getData()); } } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessage.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageData.java similarity index 73% rename from consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessage.java rename to consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageData.java index 18aa65601e..aa59053723 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessage.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageData.java @@ -18,16 +18,16 @@ import tech.pegasys.pantheon.ethereum.rlp.RLP; import tech.pegasys.pantheon.util.bytes.BytesValue; -public class CommitMessage extends AbstractIbftMessage { +public class CommitMessageData extends AbstractIbftMessageData { private static final int MESSAGE_CODE = IbftV2.COMMIT; - private CommitMessage(final BytesValue data) { + private CommitMessageData(final BytesValue data) { super(data); } - public static CommitMessage fromMessage(final MessageData message) { - return fromMessage(message, MESSAGE_CODE, CommitMessage.class, CommitMessage::new); + public static CommitMessageData fromMessageData(final MessageData messageData) { + return fromMessageData(messageData, MESSAGE_CODE, CommitMessageData.class, CommitMessageData::new); } @Override @@ -35,9 +35,9 @@ public SignedData decode() { return SignedData.readSignedCommitPayloadFrom(RLP.input(data)); } - public static CommitMessage create(final SignedData signedPayload) { + public static CommitMessageData create(final SignedData signedPayload) { - return new CommitMessage(signedPayload.encode()); + return new CommitMessageData(signedPayload.encode()); } @Override diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessage.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageData.java similarity index 72% rename from consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessage.java rename to consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageData.java index 5a39f15726..18e08ba715 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessage.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageData.java @@ -18,16 +18,16 @@ import tech.pegasys.pantheon.ethereum.rlp.RLP; import tech.pegasys.pantheon.util.bytes.BytesValue; -public class NewRoundMessage extends AbstractIbftMessage { +public class NewRoundMessageData extends AbstractIbftMessageData { private static final int MESSAGE_CODE = IbftV2.NEW_ROUND; - private NewRoundMessage(final BytesValue data) { + private NewRoundMessageData(final BytesValue data) { super(data); } - public static NewRoundMessage fromMessage(final MessageData message) { - return fromMessage(message, MESSAGE_CODE, NewRoundMessage.class, NewRoundMessage::new); + public static NewRoundMessageData fromMessageData(final MessageData messageData) { + return fromMessageData(messageData, MESSAGE_CODE, NewRoundMessageData.class, NewRoundMessageData::new); } @Override @@ -35,9 +35,9 @@ public SignedData decode() { return SignedData.readSignedNewRoundPayloadFrom(RLP.input(data)); } - public static NewRoundMessage create(final SignedData signedPayload) { + public static NewRoundMessageData create(final SignedData signedPayload) { - return new NewRoundMessage(signedPayload.encode()); + return new NewRoundMessageData(signedPayload.encode()); } @Override diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessage.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageData.java similarity index 72% rename from consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessage.java rename to consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageData.java index 5549d85fe4..76f9d31d7b 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessage.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageData.java @@ -18,16 +18,16 @@ import tech.pegasys.pantheon.ethereum.rlp.RLP; import tech.pegasys.pantheon.util.bytes.BytesValue; -public class PrepareMessage extends AbstractIbftMessage { +public class PrepareMessageData extends AbstractIbftMessageData { private static final int MESSAGE_CODE = IbftV2.PREPARE; - private PrepareMessage(final BytesValue data) { + private PrepareMessageData(final BytesValue data) { super(data); } - public static PrepareMessage fromMessage(final MessageData message) { - return fromMessage(message, MESSAGE_CODE, PrepareMessage.class, PrepareMessage::new); + public static PrepareMessageData fromMessageData(final MessageData messageData) { + return fromMessageData(messageData, MESSAGE_CODE, PrepareMessageData.class, PrepareMessageData::new); } @Override @@ -35,9 +35,9 @@ public SignedData decode() { return SignedData.readSignedPreparePayloadFrom(RLP.input(data)); } - public static PrepareMessage create(final SignedData signedPayload) { + public static PrepareMessageData create(final SignedData signedPayload) { - return new PrepareMessage(signedPayload.encode()); + return new PrepareMessageData(signedPayload.encode()); } @Override diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/ProposalMessage.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/ProposalMessageData.java similarity index 72% rename from consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/ProposalMessage.java rename to consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/ProposalMessageData.java index 951e0562ad..098b1fc33b 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/ProposalMessage.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/ProposalMessageData.java @@ -18,16 +18,16 @@ import tech.pegasys.pantheon.ethereum.rlp.RLP; import tech.pegasys.pantheon.util.bytes.BytesValue; -public class ProposalMessage extends AbstractIbftMessage { +public class ProposalMessageData extends AbstractIbftMessageData { private static final int MESSAGE_CODE = IbftV2.PROPOSAL; - private ProposalMessage(final BytesValue data) { + private ProposalMessageData(final BytesValue data) { super(data); } - public static ProposalMessage fromMessage(final MessageData message) { - return fromMessage(message, MESSAGE_CODE, ProposalMessage.class, ProposalMessage::new); + public static ProposalMessageData fromMessageData(final MessageData messageData) { + return fromMessageData(messageData, MESSAGE_CODE, ProposalMessageData.class, ProposalMessageData::new); } @Override @@ -35,9 +35,9 @@ public SignedData decode() { return SignedData.readSignedProposalPayloadFrom(RLP.input(data)); } - public static ProposalMessage create(final SignedData signedPayload) { + public static ProposalMessageData create(final SignedData signedPayload) { - return new ProposalMessage(signedPayload.encode()); + return new ProposalMessageData(signedPayload.encode()); } @Override diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessage.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageData.java similarity index 71% rename from consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessage.java rename to consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageData.java index 6fcf934c91..52944d6170 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessage.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageData.java @@ -18,16 +18,16 @@ import tech.pegasys.pantheon.ethereum.rlp.RLP; import tech.pegasys.pantheon.util.bytes.BytesValue; -public class RoundChangeMessage extends AbstractIbftMessage { +public class RoundChangeMessageData extends AbstractIbftMessageData { private static final int MESSAGE_CODE = IbftV2.ROUND_CHANGE; - private RoundChangeMessage(final BytesValue data) { + private RoundChangeMessageData(final BytesValue data) { super(data); } - public static RoundChangeMessage fromMessage(final MessageData message) { - return fromMessage(message, MESSAGE_CODE, RoundChangeMessage.class, RoundChangeMessage::new); + public static RoundChangeMessageData fromMessageData(final MessageData messageData) { + return fromMessageData(messageData, MESSAGE_CODE, RoundChangeMessageData.class, RoundChangeMessageData::new); } @Override @@ -35,9 +35,9 @@ public SignedData decode() { return SignedData.readSignedRoundChangePayloadFrom(RLP.input(data)); } - public static RoundChangeMessage create(final SignedData signedPayload) { + public static RoundChangeMessageData create(final SignedData signedPayload) { - return new RoundChangeMessage(signedPayload.encode()); + return new RoundChangeMessageData(signedPayload.encode()); } @Override diff --git 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 index 19983f7205..2d6926ca28 100644 --- 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 @@ -19,12 +19,12 @@ import tech.pegasys.pantheon.consensus.ibft.ibftevent.IbftReceivedMessageEvent; import tech.pegasys.pantheon.consensus.ibft.ibftevent.NewChainHead; import tech.pegasys.pantheon.consensus.ibft.ibftevent.RoundExpiry; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessage; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.IbftV2; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessage; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.CommitPayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.NewRoundPayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.Payload; @@ -84,7 +84,7 @@ private void handleMessage(final MessageData messageData) { switch (messageData.getCode()) { case IbftV2.PROPOSAL: final SignedData proposalMsg = - ProposalMessage.fromMessage(messageData).decode(); + ProposalMessageData.fromMessageData(messageData).decode(); if (processMessage(proposalMsg, messageData)) { currentHeightManager.handleProposalMessage(proposalMsg); } @@ -92,14 +92,14 @@ private void handleMessage(final MessageData messageData) { case IbftV2.PREPARE: final SignedData prepareMsg = - PrepareMessage.fromMessage(messageData).decode(); + PrepareMessageData.fromMessageData(messageData).decode(); if (processMessage(prepareMsg, messageData)) { currentHeightManager.handlePrepareMessage(prepareMsg); } break; case IbftV2.COMMIT: - final SignedData commitMsg = CommitMessage.fromMessage(messageData).decode(); + final SignedData commitMsg = CommitMessageData.fromMessageData(messageData).decode(); if (processMessage(commitMsg, messageData)) { currentHeightManager.handleCommitMessage(commitMsg); } @@ -107,7 +107,7 @@ private void handleMessage(final MessageData messageData) { case IbftV2.ROUND_CHANGE: final SignedData roundChangeMsg = - RoundChangeMessage.fromMessage(messageData).decode(); + RoundChangeMessageData.fromMessageData(messageData).decode(); if (processMessage(roundChangeMsg, messageData)) { currentHeightManager.handleRoundChangeMessage(roundChangeMsg); } @@ -115,7 +115,7 @@ private void handleMessage(final MessageData messageData) { case IbftV2.NEW_ROUND: final SignedData newRoundMsg = - NewRoundMessage.fromMessage(messageData).decode(); + NewRoundMessageData.fromMessageData(messageData).decode(); if (processMessage(newRoundMsg, messageData)) { currentHeightManager.handleNewRoundMessage(newRoundMsg); } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftMessageTransmitter.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftMessageTransmitter.java index d91aad559a..5f382ae518 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftMessageTransmitter.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftMessageTransmitter.java @@ -13,11 +13,11 @@ package tech.pegasys.pantheon.consensus.ibft.statemachine; import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessage; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.CommitPayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.MessageFactory; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.NewRoundPayload; @@ -49,7 +49,7 @@ public void multicastProposal(final ConsensusRoundIdentifier roundIdentifier, fi final SignedData signedPayload = messageFactory.createSignedProposalPayload(roundIdentifier, block); - final ProposalMessage message = ProposalMessage.create(signedPayload); + final ProposalMessageData message = ProposalMessageData.create(signedPayload); networkPeers.multicastToValidators(message); } @@ -58,7 +58,7 @@ public void multicastPrepare(final ConsensusRoundIdentifier roundIdentifier, fin final SignedData signedPayload = messageFactory.createSignedPreparePayload(roundIdentifier, digest); - final PrepareMessage message = PrepareMessage.create(signedPayload); + final PrepareMessageData message = PrepareMessageData.create(signedPayload); networkPeers.multicastToValidators(message); } @@ -70,7 +70,7 @@ public void multicastCommit( final SignedData signedPayload = messageFactory.createSignedCommitPayload(roundIdentifier, digest, commitSeal); - final CommitMessage message = CommitMessage.create(signedPayload); + final CommitMessageData message = CommitMessageData.create(signedPayload); networkPeers.multicastToValidators(message); } @@ -82,7 +82,7 @@ public void multicastRoundChange( final SignedData signedPayload = messageFactory.createSignedRoundChangePayload(roundIdentifier, preparedCertificate); - final RoundChangeMessage message = RoundChangeMessage.create(signedPayload); + final RoundChangeMessageData message = RoundChangeMessageData.create(signedPayload); networkPeers.multicastToValidators(message); } @@ -96,7 +96,7 @@ public void multicastNewRound( messageFactory.createSignedNewRoundPayload( roundIdentifier, roundChangeCertificate, proposalPayload); - final NewRoundMessage message = NewRoundMessage.create(signedPayload); + final NewRoundMessageData message = NewRoundMessageData.create(signedPayload); networkPeers.multicastToValidators(message); } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageTest.java index 01ef9a5bb6..d9b7f16832 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageTest.java @@ -32,12 +32,12 @@ public class CommitMessageTest { @Mock private SignedData commitPayload; @Mock private BytesValue messageBytes; @Mock private MessageData messageData; - @Mock private CommitMessage commitMessage; + @Mock private CommitMessageData commitMessage; @Test public void createMessageFromCommitMessageData() { when(commitPayload.encode()).thenReturn(messageBytes); - CommitMessage commitMessage = CommitMessage.create(commitPayload); + CommitMessageData commitMessage = CommitMessageData.create(commitPayload); assertThat(commitMessage.getData()).isEqualTo(messageBytes); assertThat(commitMessage.getCode()).isEqualTo(IbftV2.COMMIT); @@ -46,7 +46,7 @@ public void createMessageFromCommitMessageData() { @Test public void createMessageFromCommitMessage() { - CommitMessage message = CommitMessage.fromMessage(commitMessage); + CommitMessageData message = CommitMessageData.fromMessageData(commitMessage); assertThat(message).isSameAs(commitMessage); } @@ -54,7 +54,7 @@ public void createMessageFromCommitMessage() { public void createMessageFromGenericMessageData() { when(messageData.getData()).thenReturn(messageBytes); when(messageData.getCode()).thenReturn(IbftV2.COMMIT); - CommitMessage commitMessage = CommitMessage.fromMessage(messageData); + CommitMessageData commitMessage = CommitMessageData.fromMessageData(messageData); assertThat(commitMessage.getData()).isEqualTo(messageData.getData()); assertThat(commitMessage.getCode()).isEqualTo(IbftV2.COMMIT); @@ -63,8 +63,8 @@ public void createMessageFromGenericMessageData() { @Test public void createMessageFailsWhenIncorrectMessageCode() { when(messageData.getCode()).thenReturn(42); - assertThatThrownBy(() -> CommitMessage.fromMessage(messageData)) + assertThatThrownBy(() -> CommitMessageData.fromMessageData(messageData)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Message has code 42 and thus is not a CommitMessage"); + .hasMessageContaining("Message has code 42 and thus is not a CommitMessageData"); } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageTest.java index e43cfc3b14..3b2ee33c53 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageTest.java @@ -32,12 +32,12 @@ public class NewRoundMessageTest { @Mock private SignedData newRoundPayload; @Mock private BytesValue messageBytes; @Mock private MessageData messageData; - @Mock private NewRoundMessage newRoundMessage; + @Mock private NewRoundMessageData newRoundMessage; @Test public void createMessageFromNewRoundChangeMessageData() { when(newRoundPayload.encode()).thenReturn(messageBytes); - NewRoundMessage prepareMessage = NewRoundMessage.create(newRoundPayload); + NewRoundMessageData prepareMessage = NewRoundMessageData.create(newRoundPayload); assertThat(prepareMessage.getData()).isEqualTo(messageBytes); assertThat(prepareMessage.getCode()).isEqualTo(IbftV2.NEW_ROUND); @@ -46,7 +46,7 @@ public void createMessageFromNewRoundChangeMessageData() { @Test public void createMessageFromNewRoundMessage() { - NewRoundMessage message = NewRoundMessage.fromMessage(newRoundMessage); + NewRoundMessageData message = NewRoundMessageData.fromMessageData(newRoundMessage); assertThat(message).isSameAs(newRoundMessage); } @@ -54,7 +54,7 @@ public void createMessageFromNewRoundMessage() { public void createMessageFromGenericMessageData() { when(messageData.getData()).thenReturn(messageBytes); when(messageData.getCode()).thenReturn(IbftV2.NEW_ROUND); - NewRoundMessage newRoundMessage = NewRoundMessage.fromMessage(messageData); + NewRoundMessageData newRoundMessage = NewRoundMessageData.fromMessageData(messageData); assertThat(newRoundMessage.getData()).isEqualTo(messageData.getData()); assertThat(newRoundMessage.getCode()).isEqualTo(IbftV2.NEW_ROUND); @@ -63,8 +63,8 @@ public void createMessageFromGenericMessageData() { @Test public void createMessageFailsWhenIncorrectMessageCode() { when(messageData.getCode()).thenReturn(42); - assertThatThrownBy(() -> NewRoundMessage.fromMessage(messageData)) + assertThatThrownBy(() -> NewRoundMessageData.fromMessageData(messageData)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Message has code 42 and thus is not a NewRoundMessage"); + .hasMessageContaining("Message has code 42 and thus is not a NewRoundMessageData"); } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrePrepareMessageTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrePrepareMessageTest.java index 44fdecf769..5a1bdf630c 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrePrepareMessageTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrePrepareMessageTest.java @@ -32,12 +32,12 @@ public class PrePrepareMessageTest { @Mock private SignedData prePrepareMessageData; @Mock private BytesValue messageBytes; @Mock private MessageData messageData; - @Mock private ProposalMessage proposalMessage; + @Mock private ProposalMessageData proposalMessage; @Test public void createMessageFromPrePrepareMessageData() { when(prePrepareMessageData.encode()).thenReturn(messageBytes); - ProposalMessage proposalMessage = ProposalMessage.create(prePrepareMessageData); + ProposalMessageData proposalMessage = ProposalMessageData.create(prePrepareMessageData); assertThat(proposalMessage.getData()).isEqualTo(messageBytes); assertThat(proposalMessage.getCode()).isEqualTo(IbftV2.PROPOSAL); @@ -46,7 +46,7 @@ public void createMessageFromPrePrepareMessageData() { @Test public void createMessageFromPrePrepareMessage() { - ProposalMessage message = ProposalMessage.fromMessage(proposalMessage); + ProposalMessageData message = ProposalMessageData.fromMessageData(proposalMessage); assertThat(message).isSameAs(proposalMessage); } @@ -54,7 +54,7 @@ public void createMessageFromPrePrepareMessage() { public void createMessageFromGenericMessageData() { when(messageData.getCode()).thenReturn(IbftV2.PROPOSAL); when(messageData.getData()).thenReturn(messageBytes); - ProposalMessage proposalMessage = ProposalMessage.fromMessage(messageData); + ProposalMessageData proposalMessage = ProposalMessageData.fromMessageData(messageData); assertThat(proposalMessage.getData()).isEqualTo(messageData.getData()); assertThat(proposalMessage.getCode()).isEqualTo(IbftV2.PROPOSAL); @@ -63,8 +63,8 @@ public void createMessageFromGenericMessageData() { @Test public void createMessageFailsWhenIncorrectMessageCode() { when(messageData.getCode()).thenReturn(42); - assertThatThrownBy(() -> ProposalMessage.fromMessage(messageData)) + assertThatThrownBy(() -> ProposalMessageData.fromMessageData(messageData)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Message has code 42 and thus is not a ProposalMessage"); + .hasMessageContaining("Message has code 42 and thus is not a ProposalMessageData"); } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageTest.java index 5b30449977..71a010fab7 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageTest.java @@ -32,12 +32,12 @@ public class PrepareMessageTest { @Mock private SignedData preparePayload; @Mock private BytesValue messageBytes; @Mock private MessageData messageData; - @Mock private PrepareMessage prepareMessage; + @Mock private PrepareMessageData prepareMessage; @Test public void createMessageFromPrepareMessageData() { when(preparePayload.encode()).thenReturn(messageBytes); - PrepareMessage prepareMessage = PrepareMessage.create(preparePayload); + PrepareMessageData prepareMessage = PrepareMessageData.create(preparePayload); assertThat(prepareMessage.getData()).isEqualTo(messageBytes); assertThat(prepareMessage.getCode()).isEqualTo(IbftV2.PREPARE); @@ -46,7 +46,7 @@ public void createMessageFromPrepareMessageData() { @Test public void createMessageFromPrepareMessage() { - PrepareMessage message = PrepareMessage.fromMessage(prepareMessage); + PrepareMessageData message = PrepareMessageData.fromMessageData(prepareMessage); assertThat(message).isSameAs(prepareMessage); } @@ -54,7 +54,7 @@ public void createMessageFromPrepareMessage() { public void createMessageFromGenericMessageData() { when(messageData.getData()).thenReturn(messageBytes); when(messageData.getCode()).thenReturn(IbftV2.PREPARE); - PrepareMessage prepareMessage = PrepareMessage.fromMessage(messageData); + PrepareMessageData prepareMessage = PrepareMessageData.fromMessageData(messageData); assertThat(prepareMessage.getData()).isEqualTo(messageData.getData()); assertThat(prepareMessage.getCode()).isEqualTo(IbftV2.PREPARE); @@ -63,8 +63,8 @@ public void createMessageFromGenericMessageData() { @Test public void createMessageFailsWhenIncorrectMessageCode() { when(messageData.getCode()).thenReturn(42); - assertThatThrownBy(() -> PrepareMessage.fromMessage(messageData)) + assertThatThrownBy(() -> PrepareMessageData.fromMessageData(messageData)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Message has code 42 and thus is not a PrepareMessage"); + .hasMessageContaining("Message has code 42 and thus is not a PrepareMessageData"); } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageTest.java index 71bfd6fbcb..b32219809a 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageTest.java @@ -32,12 +32,12 @@ public class RoundChangeMessageTest { @Mock private SignedData roundChangePayload; @Mock private BytesValue messageBytes; @Mock private MessageData messageData; - @Mock private RoundChangeMessage roundChangeMessage; + @Mock private RoundChangeMessageData roundChangeMessage; @Test public void createMessageFromRoundChangeMessageData() { when(roundChangePayload.encode()).thenReturn(messageBytes); - RoundChangeMessage roundChangeMessage = RoundChangeMessage.create(roundChangePayload); + RoundChangeMessageData roundChangeMessage = RoundChangeMessageData.create(roundChangePayload); assertThat(roundChangeMessage.getData()).isEqualTo(messageBytes); assertThat(roundChangeMessage.getCode()).isEqualTo(IbftV2.ROUND_CHANGE); @@ -46,7 +46,7 @@ public void createMessageFromRoundChangeMessageData() { @Test public void createMessageFromRoundChangeMessage() { - RoundChangeMessage message = RoundChangeMessage.fromMessage(roundChangeMessage); + RoundChangeMessageData message = RoundChangeMessageData.fromMessageData(roundChangeMessage); assertThat(message).isSameAs(roundChangeMessage); } @@ -54,7 +54,7 @@ public void createMessageFromRoundChangeMessage() { public void createMessageFromGenericMessageData() { when(messageData.getData()).thenReturn(messageBytes); when(messageData.getCode()).thenReturn(IbftV2.ROUND_CHANGE); - RoundChangeMessage roundChangeMessage = RoundChangeMessage.fromMessage(messageData); + RoundChangeMessageData roundChangeMessage = RoundChangeMessageData.fromMessageData(messageData); assertThat(roundChangeMessage.getData()).isEqualTo(messageData.getData()); assertThat(roundChangeMessage.getCode()).isEqualTo(IbftV2.ROUND_CHANGE); @@ -63,8 +63,8 @@ public void createMessageFromGenericMessageData() { @Test public void createMessageFailsWhenIncorrectMessageCode() { when(messageData.getCode()).thenReturn(42); - assertThatThrownBy(() -> RoundChangeMessage.fromMessage(messageData)) + assertThatThrownBy(() -> RoundChangeMessageData.fromMessageData(messageData)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Message has code 42 and thus is not a RoundChangeMessage"); + .hasMessageContaining("Message has code 42 and thus is not a RoundChangeMessageData"); } } diff --git 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 index 2ebe78f0cd..2f929f9efd 100644 --- 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 @@ -26,12 +26,12 @@ import tech.pegasys.pantheon.consensus.ibft.ibftevent.IbftReceivedMessageEvent; import tech.pegasys.pantheon.consensus.ibft.ibftevent.NewChainHead; import tech.pegasys.pantheon.consensus.ibft.ibftevent.RoundExpiry; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessage; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.IbftV2; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessage; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.CommitPayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.NewRoundPayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparePayload; @@ -64,23 +64,23 @@ public class IbftControllerTest { @Mock private IbftBlockHeightManager blockHeightManager; @Mock private SignedData signedProposal; - @Mock private ProposalMessage proposalMessage; + @Mock private ProposalMessageData proposalMessage; @Mock private ProposalPayload proposalPayload; @Mock private SignedData signedPrepare; - @Mock private PrepareMessage prepareMessage; + @Mock private PrepareMessageData prepareMessage; @Mock private PreparePayload preparePayload; @Mock private SignedData signedCommit; - @Mock private CommitMessage commitMessage; + @Mock private CommitMessageData commitMessage; @Mock private CommitPayload commitPayload; @Mock private SignedData signedNewRound; - @Mock private NewRoundMessage newRoundMessage; + @Mock private NewRoundMessageData newRoundMessage; @Mock private NewRoundPayload newRoundPayload; @Mock private SignedData signedRoundChange; - @Mock private RoundChangeMessage roundChangeMessage; + @Mock private RoundChangeMessageData roundChangeMessage; @Mock private RoundChangePayload roundChangePayload; private final Map> futureMessages = new HashMap<>(); From 4550cc0f3e31c0533e3c10b446196e24df618d1a Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 20 Dec 2018 10:25:09 +1000 Subject: [PATCH 02/33] [NC-1909] handle message and future buffer now deal with messages not messageData's --- .../consensus/ibft/ibftevent/IbftEvents.java | 2 +- .../ibftevent/IbftReceivedMessageEvent.java | 13 +++- .../ibftmessage/AbstractIbftMessageData.java | 3 +- .../ibft/ibftmessage/CommitMessageData.java | 3 +- .../ibft/ibftmessage/NewRoundMessageData.java | 3 +- .../ibft/ibftmessage/PrepareMessageData.java | 3 +- .../ibft/ibftmessage/ProposalMessageData.java | 3 +- .../ibftmessage/RoundChangeMessageData.java | 3 +- .../ibft/statemachine/IbftController.java | 30 ++++---- .../ibft/statemachine/IbftControllerTest.java | 76 ++++++++++++------- 10 files changed, 88 insertions(+), 51 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftEvents.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftEvents.java index ea91fb8f96..15f0770e0d 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftEvents.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftEvents.java @@ -17,7 +17,7 @@ /** Static helper functions for producing and working with IbftEvent objects */ public class IbftEvents { public static IbftEvent fromMessage(final Message message) { - return new IbftReceivedMessageEvent(message.getData()); + return new IbftReceivedMessageEvent(message); } public enum Type { diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftReceivedMessageEvent.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftReceivedMessageEvent.java index 332f559ea8..aac285b273 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftReceivedMessageEvent.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftReceivedMessageEvent.java @@ -13,18 +13,23 @@ package tech.pegasys.pantheon.consensus.ibft.ibftevent; import tech.pegasys.pantheon.consensus.ibft.ibftevent.IbftEvents.Type; +import tech.pegasys.pantheon.ethereum.p2p.api.Message; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; public class IbftReceivedMessageEvent implements IbftEvent { - private final MessageData messageData; + private final Message message; - public IbftReceivedMessageEvent(final MessageData messageData) { - this.messageData = messageData; + public IbftReceivedMessageEvent(final Message message) { + this.message = message; } public MessageData getMessageData() { - return messageData; + return message.getData(); + } + + public Message getMessage() { + return message; } @Override diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java index 0970b40bd9..2f3aa8d28a 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java @@ -39,7 +39,8 @@ protected static T fromMessageData( final int code = messageData.getCode(); if (code != messageCode) { throw new IllegalArgumentException( - String.format("MessageData has code %d and thus is not a %s", code, clazz.getSimpleName())); + String.format( + "MessageData has code %d and thus is not a %s", code, clazz.getSimpleName())); } return constructor.apply(messageData.getData()); diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageData.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageData.java index aa59053723..eec605c15b 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageData.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageData.java @@ -27,7 +27,8 @@ private CommitMessageData(final BytesValue data) { } public static CommitMessageData fromMessageData(final MessageData messageData) { - return fromMessageData(messageData, MESSAGE_CODE, CommitMessageData.class, CommitMessageData::new); + return fromMessageData( + messageData, MESSAGE_CODE, CommitMessageData.class, CommitMessageData::new); } @Override diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageData.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageData.java index 18e08ba715..7cafec1db3 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageData.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageData.java @@ -27,7 +27,8 @@ private NewRoundMessageData(final BytesValue data) { } public static NewRoundMessageData fromMessageData(final MessageData messageData) { - return fromMessageData(messageData, MESSAGE_CODE, NewRoundMessageData.class, NewRoundMessageData::new); + return fromMessageData( + messageData, MESSAGE_CODE, NewRoundMessageData.class, NewRoundMessageData::new); } @Override diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageData.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageData.java index 76f9d31d7b..dfe0248f3e 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageData.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageData.java @@ -27,7 +27,8 @@ private PrepareMessageData(final BytesValue data) { } public static PrepareMessageData fromMessageData(final MessageData messageData) { - return fromMessageData(messageData, MESSAGE_CODE, PrepareMessageData.class, PrepareMessageData::new); + return fromMessageData( + messageData, MESSAGE_CODE, PrepareMessageData.class, PrepareMessageData::new); } @Override diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/ProposalMessageData.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/ProposalMessageData.java index 098b1fc33b..b179b2a739 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/ProposalMessageData.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/ProposalMessageData.java @@ -27,7 +27,8 @@ private ProposalMessageData(final BytesValue data) { } public static ProposalMessageData fromMessageData(final MessageData messageData) { - return fromMessageData(messageData, MESSAGE_CODE, ProposalMessageData.class, ProposalMessageData::new); + return fromMessageData( + messageData, MESSAGE_CODE, ProposalMessageData.class, ProposalMessageData::new); } @Override diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageData.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageData.java index 52944d6170..40425bf8e3 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageData.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageData.java @@ -27,7 +27,8 @@ private RoundChangeMessageData(final BytesValue data) { } public static RoundChangeMessageData fromMessageData(final MessageData messageData) { - return fromMessageData(messageData, MESSAGE_CODE, RoundChangeMessageData.class, RoundChangeMessageData::new); + return fromMessageData( + messageData, MESSAGE_CODE, RoundChangeMessageData.class, RoundChangeMessageData::new); } @Override diff --git 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 index 2d6926ca28..5669b20e95 100644 --- 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 @@ -34,6 +34,7 @@ import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; import tech.pegasys.pantheon.ethereum.chain.Blockchain; import tech.pegasys.pantheon.ethereum.core.BlockHeader; +import tech.pegasys.pantheon.ethereum.p2p.api.Message; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; import java.util.List; @@ -50,7 +51,7 @@ public class IbftController { private final Blockchain blockchain; private final IbftFinalState ibftFinalState; private final IbftBlockHeightManagerFactory ibftBlockHeightManagerFactory; - private final Map> futureMessages; + private final Map> futureMessages; private IbftBlockHeightManager currentHeightManager; public IbftController( @@ -65,7 +66,7 @@ public IbftController( final Blockchain blockchain, final IbftFinalState ibftFinalState, final IbftBlockHeightManagerFactory ibftBlockHeightManagerFactory, - final Map> futureMessages) { + final Map> futureMessages) { this.blockchain = blockchain; this.ibftFinalState = ibftFinalState; this.ibftBlockHeightManagerFactory = ibftBlockHeightManagerFactory; @@ -77,15 +78,16 @@ public void start() { } public void handleMessageEvent(final IbftReceivedMessageEvent msg) { - handleMessage(msg.getMessageData()); + handleMessage(msg.getMessage()); } - private void handleMessage(final MessageData messageData) { + private void handleMessage(final Message message) { + final MessageData messageData = message.getData(); switch (messageData.getCode()) { case IbftV2.PROPOSAL: final SignedData proposalMsg = ProposalMessageData.fromMessageData(messageData).decode(); - if (processMessage(proposalMsg, messageData)) { + if (processMessage(proposalMsg, message)) { currentHeightManager.handleProposalMessage(proposalMsg); } break; @@ -93,14 +95,15 @@ private void handleMessage(final MessageData messageData) { case IbftV2.PREPARE: final SignedData prepareMsg = PrepareMessageData.fromMessageData(messageData).decode(); - if (processMessage(prepareMsg, messageData)) { + if (processMessage(prepareMsg, message)) { currentHeightManager.handlePrepareMessage(prepareMsg); } break; case IbftV2.COMMIT: - final SignedData commitMsg = CommitMessageData.fromMessageData(messageData).decode(); - if (processMessage(commitMsg, messageData)) { + final SignedData commitMsg = + CommitMessageData.fromMessageData(messageData).decode(); + if (processMessage(commitMsg, message)) { currentHeightManager.handleCommitMessage(commitMsg); } break; @@ -108,7 +111,7 @@ private void handleMessage(final MessageData messageData) { case IbftV2.ROUND_CHANGE: final SignedData roundChangeMsg = RoundChangeMessageData.fromMessageData(messageData).decode(); - if (processMessage(roundChangeMsg, messageData)) { + if (processMessage(roundChangeMsg, message)) { currentHeightManager.handleRoundChangeMessage(roundChangeMsg); } break; @@ -116,7 +119,7 @@ private void handleMessage(final MessageData messageData) { case IbftV2.NEW_ROUND: final SignedData newRoundMsg = NewRoundMessageData.fromMessageData(messageData).decode(); - if (processMessage(newRoundMsg, messageData)) { + if (processMessage(newRoundMsg, message)) { currentHeightManager.handleNewRoundMessage(newRoundMsg); } break; @@ -151,13 +154,12 @@ private void startNewHeightManager(final BlockHeader parentHeader) { currentHeightManager = ibftBlockHeightManagerFactory.create(parentHeader); currentHeightManager.start(); final long newChainHeight = currentHeightManager.getChainHeight(); - List orDefault = futureMessages.getOrDefault(newChainHeight, emptyList()); + List orDefault = futureMessages.getOrDefault(newChainHeight, emptyList()); orDefault.forEach(this::handleMessage); futureMessages.remove(newChainHeight); } - private boolean processMessage( - final SignedData msg, final MessageData rawMsg) { + private boolean processMessage(final SignedData msg, final Message rawMsg) { final ConsensusRoundIdentifier msgRoundIdentifier = msg.getPayload().getRoundIdentifier(); if (isMsgForCurrentHeight(msgRoundIdentifier)) { return isMsgFromKnownValidator(msg); @@ -181,7 +183,7 @@ private boolean isMsgForFutureChainHeight(final ConsensusRoundIdentifier roundId return roundIdentifier.getSequenceNumber() > currentHeightManager.getChainHeight(); } - private void addMessageToFutureMessageBuffer(final long chainHeight, final MessageData rawMsg) { + private void addMessageToFutureMessageBuffer(final long chainHeight, final Message rawMsg) { if (!futureMessages.containsKey(chainHeight)) { futureMessages.put(chainHeight, Lists.newArrayList()); } diff --git 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 index 2f929f9efd..d52077a2b3 100644 --- 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 @@ -41,11 +41,14 @@ import tech.pegasys.pantheon.ethereum.chain.Blockchain; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; -import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; +import tech.pegasys.pantheon.ethereum.p2p.api.Message; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import javafx.util.Pair; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -64,26 +67,31 @@ public class IbftControllerTest { @Mock private IbftBlockHeightManager blockHeightManager; @Mock private SignedData signedProposal; - @Mock private ProposalMessageData proposalMessage; + @Mock private Message proposalMessage; + @Mock private ProposalMessageData proposalMessageData; @Mock private ProposalPayload proposalPayload; @Mock private SignedData signedPrepare; - @Mock private PrepareMessageData prepareMessage; + @Mock private Message prepareMessage; + @Mock private PrepareMessageData prepareMessageData; @Mock private PreparePayload preparePayload; @Mock private SignedData signedCommit; - @Mock private CommitMessageData commitMessage; + @Mock private Message commitMessage; + @Mock private CommitMessageData commitMessageData; @Mock private CommitPayload commitPayload; @Mock private SignedData signedNewRound; - @Mock private NewRoundMessageData newRoundMessage; + @Mock private Message newRoundMessage; + @Mock private NewRoundMessageData newRoundMessageData; @Mock private NewRoundPayload newRoundPayload; @Mock private SignedData signedRoundChange; - @Mock private RoundChangeMessageData roundChangeMessage; + @Mock private Message roundChangeMessage; + @Mock private RoundChangeMessageData roundChangeMessageData; @Mock private RoundChangePayload roundChangePayload; - private final Map> futureMessages = new HashMap<>(); + private final Map> futureMessages = new HashMap<>(); private final Address validator = Address.fromHexString("0x0"); private final Address unknownValidator = Address.fromHexString("0x2"); private final ConsensusRoundIdentifier futureRoundIdentifier = new ConsensusRoundIdentifier(2, 0); @@ -115,9 +123,9 @@ public void startsNewBlockHeightManagerAndReplaysFutureMessages() { setupRoundChange(futureRoundIdentifier, validator); setupNewRound(roundIdentifierHeight3, validator); - final List height2Msgs = + final List height2Msgs = newArrayList(prepareMessage, commitMessage, roundChangeMessage); - final List height3Msgs = newArrayList(proposalMessage, newRoundMessage); + final List height3Msgs = newArrayList(proposalMessage, newRoundMessage); futureMessages.put(2L, height2Msgs); futureMessages.put(3L, height3Msgs); when(blockHeightManager.getChainHeight()).thenReturn(2L); @@ -337,7 +345,7 @@ public void roundChangeForUnknownValidatorIsDiscarded() { @Test public void proposalForFutureHeightIsBuffered() { setupProposal(futureRoundIdentifier, validator); - final Map> expectedFutureMsgs = + final Map> expectedFutureMsgs = ImmutableMap.of(2L, ImmutableList.of(proposalMessage)); verifyHasFutureMessages(new IbftReceivedMessageEvent(proposalMessage), expectedFutureMsgs); } @@ -345,7 +353,7 @@ public void proposalForFutureHeightIsBuffered() { @Test public void prepareForFutureHeightIsBuffered() { setupPrepare(futureRoundIdentifier, validator); - final Map> expectedFutureMsgs = + final Map> expectedFutureMsgs = ImmutableMap.of(2L, ImmutableList.of(prepareMessage)); verifyHasFutureMessages(new IbftReceivedMessageEvent(prepareMessage), expectedFutureMsgs); } @@ -353,7 +361,7 @@ public void prepareForFutureHeightIsBuffered() { @Test public void commitForFutureHeightIsBuffered() { setupCommit(futureRoundIdentifier, validator); - final Map> expectedFutureMsgs = + final Map> expectedFutureMsgs = ImmutableMap.of(2L, ImmutableList.of(commitMessage)); verifyHasFutureMessages(new IbftReceivedMessageEvent(commitMessage), expectedFutureMsgs); } @@ -361,7 +369,7 @@ public void commitForFutureHeightIsBuffered() { @Test public void newRoundForFutureHeightIsBuffered() { setupNewRound(futureRoundIdentifier, validator); - final Map> expectedFutureMsgs = + final Map> expectedFutureMsgs = ImmutableMap.of(2L, ImmutableList.of(newRoundMessage)); verifyHasFutureMessages(new IbftReceivedMessageEvent(newRoundMessage), expectedFutureMsgs); } @@ -369,7 +377,7 @@ public void newRoundForFutureHeightIsBuffered() { @Test public void roundChangeForFutureHeightIsBuffered() { setupRoundChange(futureRoundIdentifier, validator); - final Map> expectedFutureMsgs = + final Map> expectedFutureMsgs = ImmutableMap.of(2L, ImmutableList.of(roundChangeMessage)); verifyHasFutureMessages(new IbftReceivedMessageEvent(roundChangeMessage), expectedFutureMsgs); } @@ -384,13 +392,24 @@ private void verifyNotHandledAndNoFutureMsgs(final IbftReceivedMessageEvent msg) verifyNoMoreInteractions(blockHeightManager); } + public static List> zip(List as, List bs) { + return IntStream.range(0, Math.min(as.size(), bs.size())) + .mapToObj(i -> new Pair<>(as.get(i), bs.get(i))) + .collect(Collectors.toList()); + } + private void verifyHasFutureMessages( - final IbftReceivedMessageEvent msg, final Map> expectedFutureMsgs) { + final IbftReceivedMessageEvent msg, final Map> expectedFutureMsgs) { ibftController.start(); ibftController.handleMessageEvent(msg); assertThat(futureMessages).hasSize(expectedFutureMsgs.size()); - assertThat(futureMessages).isEqualTo(expectedFutureMsgs); + assertThat(futureMessages) + .allSatisfy( + (round, messages) -> + assertThat(zip(expectedFutureMsgs.get(round), messages)) + .allSatisfy( + (m) -> assertThat(m.getKey().getData()).isEqualTo(m.getValue().getData()))); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -401,8 +420,9 @@ private void setupProposal( when(signedProposal.getPayload()).thenReturn(proposalPayload); when(signedProposal.getSender()).thenReturn(validator); when(proposalPayload.getRoundIdentifier()).thenReturn(roundIdentifier); - when(proposalMessage.getCode()).thenReturn(IbftV2.PROPOSAL); - when(proposalMessage.decode()).thenReturn(signedProposal); + when(proposalMessageData.getCode()).thenReturn(IbftV2.PROPOSAL); + when(proposalMessageData.decode()).thenReturn(signedProposal); + when(proposalMessage.getData()).thenReturn(proposalMessageData); } private void setupPrepare( @@ -410,8 +430,9 @@ private void setupPrepare( when(signedPrepare.getPayload()).thenReturn(preparePayload); when(signedPrepare.getSender()).thenReturn(validator); when(preparePayload.getRoundIdentifier()).thenReturn(roundIdentifier); - when(prepareMessage.getCode()).thenReturn(IbftV2.PREPARE); - when(prepareMessage.decode()).thenReturn(signedPrepare); + when(prepareMessageData.getCode()).thenReturn(IbftV2.PREPARE); + when(prepareMessageData.decode()).thenReturn(signedPrepare); + when(prepareMessage.getData()).thenReturn(prepareMessageData); } private void setupCommit( @@ -419,8 +440,9 @@ private void setupCommit( when(signedCommit.getPayload()).thenReturn(commitPayload); when(signedCommit.getSender()).thenReturn(validator); when(commitPayload.getRoundIdentifier()).thenReturn(roundIdentifier); - when(commitMessage.getCode()).thenReturn(IbftV2.COMMIT); - when(commitMessage.decode()).thenReturn(signedCommit); + when(commitMessageData.getCode()).thenReturn(IbftV2.COMMIT); + when(commitMessageData.decode()).thenReturn(signedCommit); + when(commitMessage.getData()).thenReturn(commitMessageData); } private void setupNewRound( @@ -428,8 +450,9 @@ private void setupNewRound( when(signedNewRound.getPayload()).thenReturn(newRoundPayload); when(signedNewRound.getSender()).thenReturn(validator); when(newRoundPayload.getRoundIdentifier()).thenReturn(roundIdentifier); - when(newRoundMessage.getCode()).thenReturn(IbftV2.NEW_ROUND); - when(newRoundMessage.decode()).thenReturn(signedNewRound); + when(newRoundMessageData.getCode()).thenReturn(IbftV2.NEW_ROUND); + when(newRoundMessageData.decode()).thenReturn(signedNewRound); + when(newRoundMessage.getData()).thenReturn(newRoundMessageData); } private void setupRoundChange( @@ -437,7 +460,8 @@ private void setupRoundChange( when(signedRoundChange.getPayload()).thenReturn(roundChangePayload); when(signedRoundChange.getSender()).thenReturn(validator); when(roundChangePayload.getRoundIdentifier()).thenReturn(roundIdentifier); - when(roundChangeMessage.getCode()).thenReturn(IbftV2.ROUND_CHANGE); - when(roundChangeMessage.decode()).thenReturn(signedRoundChange); + when(roundChangeMessageData.getCode()).thenReturn(IbftV2.ROUND_CHANGE); + when(roundChangeMessageData.decode()).thenReturn(signedRoundChange); + when(roundChangeMessage.getData()).thenReturn(roundChangeMessageData); } } From c3718d277d12dc6814f15b80842db5489cffdf37 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 20 Dec 2018 14:23:44 +1000 Subject: [PATCH 03/33] [NC-1909] passing messages instead --- .../statemachine/IbftBlockHeightManager.java | 35 ++++++++++--------- .../ibft/statemachine/IbftController.java | 4 +-- .../ibft/statemachine/IbftControllerTest.java | 19 +++------- 3 files changed, 24 insertions(+), 34 deletions(-) diff --git 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 index 8a0343272a..b3e6bb2206 100644 --- 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 @@ -44,6 +44,7 @@ import com.google.common.collect.Maps; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import tech.pegasys.pantheon.ethereum.p2p.api.Message; /** * Responsible for starting/clearing Consensus rounds at a given block height. One of these is @@ -149,47 +150,47 @@ public void roundExpired(final RoundExpiry expire) { handleRoundChangeMessage(localRoundChange); } - public void handleProposalMessage(final SignedData msg) { + public void handleProposalMessage(final SignedData msgData, final Message msg) { LOG.info("Received a Proposal message."); - actionOrBufferMessage(msg, currentRound::handleProposalMessage, RoundState::setProposedBlock); + actionOrBufferMessage(msgData, currentRound::handleProposalMessage, RoundState::setProposedBlock); } - public void handlePrepareMessage(final SignedData msg) { + public void handlePrepareMessage(final SignedData msgData) { LOG.info("Received a prepare message."); - actionOrBufferMessage(msg, currentRound::handlePrepareMessage, RoundState::addPrepareMessage); + actionOrBufferMessage(msgData, currentRound::handlePrepareMessage, RoundState::addPrepareMessage); } - public void handleCommitMessage(final SignedData msg) { + public void handleCommitMessage(final SignedData msgData) { LOG.info("Received a commit message."); - actionOrBufferMessage(msg, currentRound::handleCommitMessage, RoundState::addCommitMessage); + actionOrBufferMessage(msgData, currentRound::handleCommitMessage, RoundState::addCommitMessage); } private void actionOrBufferMessage( - final SignedData msg, + final SignedData msgData, final Consumer> inRoundHandler, final BiConsumer> buffer) { - final Payload payload = msg.getPayload(); + final Payload payload = msgData.getPayload(); final MessageAge messageAge = determineAgeOfPayload(payload); if (messageAge == CURRENT_ROUND) { - inRoundHandler.accept(msg); + inRoundHandler.accept(msgData); } else if (messageAge == FUTURE_ROUND) { final ConsensusRoundIdentifier msgRoundId = payload.getRoundIdentifier(); final RoundState roundstate = futureRoundStateBuffer.computeIfAbsent( msgRoundId.getRoundNumber(), k -> roundStateCreator.apply(msgRoundId)); - buffer.accept(roundstate, msg); + buffer.accept(roundstate, msgData); } } - public void handleRoundChangeMessage(final SignedData msg) { + public void handleRoundChangeMessage(final SignedData msgData) { final Optional result = - roundChangeManager.appendRoundChangeMessage(msg); - final MessageAge messageAge = determineAgeOfPayload(msg.getPayload()); + roundChangeManager.appendRoundChangeMessage(msgData); + final MessageAge messageAge = determineAgeOfPayload(msgData.getPayload()); if (messageAge == PRIOR_ROUND) { LOG.info("Received RoundChange Message for a prior round."); return; } - ConsensusRoundIdentifier targetRound = msg.getPayload().getRoundIdentifier(); + ConsensusRoundIdentifier targetRound = msgData.getPayload().getRoundIdentifier(); LOG.info("Received a RoundChange message for {}", targetRound.toString()); if (result.isPresent()) { @@ -218,8 +219,8 @@ private void startNewRound(final int roundNumber) { roundTimer.startTimer(currentRound.getRoundIdentifier()); } - public void handleNewRoundMessage(final SignedData msg) { - final NewRoundPayload payload = msg.getPayload(); + public void handleNewRoundMessage(final SignedData msgData) { + final NewRoundPayload payload = msgData.getPayload(); final MessageAge messageAge = determineAgeOfPayload(payload); if (messageAge == PRIOR_ROUND) { @@ -228,7 +229,7 @@ public void handleNewRoundMessage(final SignedData msg) { } LOG.info("Received NewRound Message for {}", payload.getRoundIdentifier().toString()); - if (newRoundMessageValidator.validateNewRoundMessage(msg)) { + if (newRoundMessageValidator.validateNewRoundMessage(msgData)) { if (messageAge == FUTURE_ROUND) { startNewRound(payload.getRoundIdentifier().getRoundNumber()); } diff --git 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 index 5669b20e95..a69602e635 100644 --- 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 @@ -88,7 +88,7 @@ private void handleMessage(final Message message) { final SignedData proposalMsg = ProposalMessageData.fromMessageData(messageData).decode(); if (processMessage(proposalMsg, message)) { - currentHeightManager.handleProposalMessage(proposalMsg); + currentHeightManager.handleProposalMessage(proposalMsg, message); } break; @@ -166,7 +166,7 @@ private boolean processMessage(final SignedData msg, final Me } else if (isMsgForFutureChainHeight(msgRoundIdentifier)) { addMessageToFutureMessageBuffer(msgRoundIdentifier.getSequenceNumber(), rawMsg); } else { - LOG.info("IBFT message discarded as it is not for the current block height"); + LOG.info("IBFT message discarded as it is from a previous block height"); } return false; } diff --git 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 index d52077a2b3..8b14a4e083 100644 --- 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 @@ -136,7 +136,7 @@ public void startsNewBlockHeightManagerAndReplaysFutureMessages() { verify(blockHeightManagerFactory).create(blockHeader); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); - verify(blockHeightManager, never()).handleProposalMessage(signedProposal); + verify(blockHeightManager, never()).handleProposalMessage(signedProposal, proposalMessage); verify(blockHeightManager).handlePrepareMessage(signedPrepare); verify(blockHeightManager).handleCommitMessage(signedCommit); verify(blockHeightManager).handleRoundChangeMessage(signedRoundChange); @@ -163,7 +163,7 @@ public void createsNewBlockHeightManagerAndReplaysFutureMessagesOnNewChainHeadEv verify(blockHeightManagerFactory).create(blockHeader); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); - verify(blockHeightManager).handleProposalMessage(signedProposal); + verify(blockHeightManager).handleProposalMessage(signedProposal, proposalMessage); verify(blockHeightManager).handlePrepareMessage(signedPrepare); verify(blockHeightManager).handleCommitMessage(signedCommit); verify(blockHeightManager).handleRoundChangeMessage(signedRoundChange); @@ -197,7 +197,7 @@ public void proposalForCurrentHeightIsPassedToBlockHeightManager() { ibftController.handleMessageEvent(new IbftReceivedMessageEvent(proposalMessage)); assertThat(futureMessages).isEmpty(); - verify(blockHeightManager).handleProposalMessage(signedProposal); + verify(blockHeightManager).handleProposalMessage(signedProposal, proposalMessage); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -392,24 +392,13 @@ private void verifyNotHandledAndNoFutureMsgs(final IbftReceivedMessageEvent msg) verifyNoMoreInteractions(blockHeightManager); } - public static List> zip(List as, List bs) { - return IntStream.range(0, Math.min(as.size(), bs.size())) - .mapToObj(i -> new Pair<>(as.get(i), bs.get(i))) - .collect(Collectors.toList()); - } - private void verifyHasFutureMessages( final IbftReceivedMessageEvent msg, final Map> expectedFutureMsgs) { ibftController.start(); ibftController.handleMessageEvent(msg); assertThat(futureMessages).hasSize(expectedFutureMsgs.size()); - assertThat(futureMessages) - .allSatisfy( - (round, messages) -> - assertThat(zip(expectedFutureMsgs.get(round), messages)) - .allSatisfy( - (m) -> assertThat(m.getKey().getData()).isEqualTo(m.getValue().getData()))); + assertThat(futureMessages).isEqualTo(expectedFutureMsgs); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); From d9df9dfc05f0114a4e271606cbe77f2a10246d91 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 20 Dec 2018 18:01:06 +1000 Subject: [PATCH 04/33] [NC-1909] Gossiper in place --- .../pantheon/consensus/ibft/IbftGossip.java | 80 +++++++++++++++++++ .../ibftmessage/AbstractIbftMessageData.java | 3 +- .../ibft/network/IbftNetworkPeers.java | 19 +++-- .../statemachine/IbftBlockHeightManager.java | 9 ++- .../ibft/statemachine/IbftController.java | 10 ++- .../ibft/statemachine/IbftControllerTest.java | 9 +-- .../pantheon/ethereum/p2p/wire/PeerInfo.java | 8 ++ 7 files changed, 118 insertions(+), 20 deletions(-) create mode 100644 consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java new file mode 100644 index 0000000000..93720399aa --- /dev/null +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java @@ -0,0 +1,80 @@ +package tech.pegasys.pantheon.consensus.ibft; + +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.CommitPayload; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.NewRoundPayload; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparePayload; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangePayload; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; +import tech.pegasys.pantheon.consensus.ibft.network.IbftNetworkPeers; +import tech.pegasys.pantheon.crypto.SECP256K1.Signature; +import tech.pegasys.pantheon.ethereum.core.Address; +import tech.pegasys.pantheon.ethereum.p2p.api.Message; +import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; + +public class IbftGossip { + private final IbftNetworkPeers peers; + + private static final int SEEN_MESSAGES_SIZE = 10_000; + private final Set seenMessages = + Collections.newSetFromMap( + new LinkedHashMap() { + protected boolean removeEldestEntry(Map.Entry eldest) { + return size() > SEEN_MESSAGES_SIZE; + } + }); + + public IbftGossip(IbftNetworkPeers peers) { + this.peers = peers; + } + + public boolean gossipProposalPayload(SignedData payload, Address... excludeAddresses) { + final ProposalMessageData messageData = ProposalMessageData.create(payload); + return gossipMessage(payload.getSignature(), messageData, payload.getSender(), excludeAddresses); + } + + public boolean gossipPreparePayload(SignedData payload, Address... excludeAddresses) { + final PrepareMessageData messageData = PrepareMessageData.create(payload); + return gossipMessage(payload.getSignature(), messageData, payload.getSender(), excludeAddresses); + } + + public boolean gossipCommitPayload(SignedData payload, Address... excludeAddresses) { + final CommitMessageData messageData = CommitMessageData.create(payload); + return gossipMessage(payload.getSignature(), messageData, payload.getSender(), excludeAddresses); + } + + public boolean gossipRoundChangePayload(SignedData payload, Address... excludeAddresses) { + final RoundChangeMessageData messageData = RoundChangeMessageData.create(payload); + return gossipMessage(payload.getSignature(), messageData, payload.getSender(), excludeAddresses); + } + + public boolean gossipNewRoundPayload(SignedData payload, Address... excludeAddresses) { + final NewRoundMessageData messageData = NewRoundMessageData.create(payload); + return gossipMessage(payload.getSignature(), messageData, payload.getSender(), excludeAddresses); + } + + private boolean gossipMessage(Signature signature, MessageData messageData, Address payloadSigner, Address... excludeAddresses) { + if (seenMessages.contains(signature)) { + return false; + } else { + final List
excludeAddressesList = Arrays.asList(excludeAddresses); + excludeAddressesList.add(payloadSigner); + peers.multicastToValidatorsExcept(messageData, excludeAddressesList); + seenMessages.add(signature); + return true; + } + } + + +} diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java index 2f3aa8d28a..21074dcefd 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java @@ -12,6 +12,7 @@ */ package tech.pegasys.pantheon.consensus.ibft.ibftmessage; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.Payload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; import tech.pegasys.pantheon.ethereum.p2p.wire.AbstractMessageData; @@ -24,7 +25,7 @@ protected AbstractIbftMessageData(final BytesValue data) { super(data); } - public abstract SignedData decode(); + public abstract SignedData decode(); protected static T fromMessageData( final MessageData messageData, diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java index fa5c15f6e2..645bab960d 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java @@ -12,6 +12,9 @@ */ package tech.pegasys.pantheon.consensus.ibft.network; +import com.google.common.collect.Lists; +import com.sun.tools.javac.util.List; +import java.util.Arrays; import tech.pegasys.pantheon.consensus.common.ValidatorProvider; import tech.pegasys.pantheon.crypto.SECP256K1.PublicKey; import tech.pegasys.pantheon.ethereum.core.Address; @@ -43,12 +46,12 @@ public IbftNetworkPeers(final ValidatorProvider validatorProvider) { } public void peerAdded(final PeerConnection newConnection) { - final Address peerAddress = getAddressFrom(newConnection); + final Address peerAddress = newConnection.getPeer().getAddress(); peerConnections.put(peerAddress, newConnection); } public void peerRemoved(final PeerConnection removedConnection) { - final Address peerAddress = getAddressFrom(removedConnection); + final Address peerAddress = removedConnection.getPeer().getAddress(); peerConnections.remove(peerAddress); } @@ -57,6 +60,12 @@ public void multicastToValidators(final MessageData message) { sendMessageToSpecificAddresses(validators, message); } + public void multicastToValidatorsExcept(final MessageData message, Collection
exceptAddresses) { + final Collection
validators = validatorProvider.getValidators(); + validators.removeIf(exceptAddresses::contains); + sendMessageToSpecificAddresses(validators, message); + } + private void sendMessageToSpecificAddresses( final Collection
recipients, final MessageData message) { recipients @@ -72,10 +81,4 @@ private void sendMessageToSpecificAddresses( } }); } - - private Address getAddressFrom(final PeerConnection connection) { - final BytesValue peerNodeId = connection.getPeer().getNodeId(); - final PublicKey remotePublicKey = PublicKey.create(peerNodeId); - return Util.publicKeyToAddress(remotePublicKey); - } } diff --git 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 index b3e6bb2206..1c29332cd0 100644 --- 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 @@ -44,7 +44,6 @@ import com.google.common.collect.Maps; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import tech.pegasys.pantheon.ethereum.p2p.api.Message; /** * Responsible for starting/clearing Consensus rounds at a given block height. One of these is @@ -150,14 +149,16 @@ public void roundExpired(final RoundExpiry expire) { handleRoundChangeMessage(localRoundChange); } - public void handleProposalMessage(final SignedData msgData, final Message msg) { + public void handleProposalMessage(final SignedData msgData) { LOG.info("Received a Proposal message."); - actionOrBufferMessage(msgData, currentRound::handleProposalMessage, RoundState::setProposedBlock); + actionOrBufferMessage( + msgData, currentRound::handleProposalMessage, RoundState::setProposedBlock); } public void handlePrepareMessage(final SignedData msgData) { LOG.info("Received a prepare message."); - actionOrBufferMessage(msgData, currentRound::handlePrepareMessage, RoundState::addPrepareMessage); + actionOrBufferMessage( + msgData, currentRound::handlePrepareMessage, RoundState::addPrepareMessage); } public void handleCommitMessage(final SignedData msgData) { diff --git 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 index a69602e635..93f810db50 100644 --- 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 @@ -15,6 +15,7 @@ import static java.util.Collections.emptyList; import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; +import tech.pegasys.pantheon.consensus.ibft.IbftGossip; import tech.pegasys.pantheon.consensus.ibft.ibftevent.BlockTimerExpiry; import tech.pegasys.pantheon.consensus.ibft.ibftevent.IbftReceivedMessageEvent; import tech.pegasys.pantheon.consensus.ibft.ibftevent.NewChainHead; @@ -53,6 +54,7 @@ public class IbftController { private final IbftBlockHeightManagerFactory ibftBlockHeightManagerFactory; private final Map> futureMessages; private IbftBlockHeightManager currentHeightManager; + private final IbftGossip gossiper; public IbftController( final Blockchain blockchain, @@ -71,6 +73,7 @@ public IbftController( this.ibftFinalState = ibftFinalState; this.ibftBlockHeightManagerFactory = ibftBlockHeightManagerFactory; this.futureMessages = futureMessages; + this.gossiper = new IbftGossip(ibftFinalState.getPeers()); } public void start() { @@ -88,7 +91,8 @@ private void handleMessage(final Message message) { final SignedData proposalMsg = ProposalMessageData.fromMessageData(messageData).decode(); if (processMessage(proposalMsg, message)) { - currentHeightManager.handleProposalMessage(proposalMsg, message); + currentHeightManager.handleProposalMessage(proposalMsg); + gossiper.gossipProposalPayload(proposalMsg, message.getConnection().getPeer().getAddress()); } break; @@ -97,6 +101,7 @@ private void handleMessage(final Message message) { PrepareMessageData.fromMessageData(messageData).decode(); if (processMessage(prepareMsg, message)) { currentHeightManager.handlePrepareMessage(prepareMsg); + gossiper.gossipPreparePayload(prepareMsg, message.getConnection().getPeer().getAddress()); } break; @@ -105,6 +110,7 @@ private void handleMessage(final Message message) { CommitMessageData.fromMessageData(messageData).decode(); if (processMessage(commitMsg, message)) { currentHeightManager.handleCommitMessage(commitMsg); + gossiper.gossipCommitPayload(commitMsg, message.getConnection().getPeer().getAddress()); } break; @@ -113,6 +119,7 @@ private void handleMessage(final Message message) { RoundChangeMessageData.fromMessageData(messageData).decode(); if (processMessage(roundChangeMsg, message)) { currentHeightManager.handleRoundChangeMessage(roundChangeMsg); + gossiper.gossipRoundChangePayload(roundChangeMsg, message.getConnection().getPeer().getAddress()); } break; @@ -121,6 +128,7 @@ private void handleMessage(final Message message) { NewRoundMessageData.fromMessageData(messageData).decode(); if (processMessage(newRoundMsg, message)) { currentHeightManager.handleNewRoundMessage(newRoundMsg); + gossiper.gossipNewRoundPayload(newRoundMsg, message.getConnection().getPeer().getAddress()); } break; diff --git 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 index 8b14a4e083..5613568ed3 100644 --- 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 @@ -46,9 +46,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; -import java.util.stream.IntStream; -import javafx.util.Pair; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -136,7 +133,7 @@ public void startsNewBlockHeightManagerAndReplaysFutureMessages() { verify(blockHeightManagerFactory).create(blockHeader); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); - verify(blockHeightManager, never()).handleProposalMessage(signedProposal, proposalMessage); + verify(blockHeightManager, never()).handleProposalMessage(signedProposal); verify(blockHeightManager).handlePrepareMessage(signedPrepare); verify(blockHeightManager).handleCommitMessage(signedCommit); verify(blockHeightManager).handleRoundChangeMessage(signedRoundChange); @@ -163,7 +160,7 @@ public void createsNewBlockHeightManagerAndReplaysFutureMessagesOnNewChainHeadEv verify(blockHeightManagerFactory).create(blockHeader); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); - verify(blockHeightManager).handleProposalMessage(signedProposal, proposalMessage); + verify(blockHeightManager).handleProposalMessage(signedProposal); verify(blockHeightManager).handlePrepareMessage(signedPrepare); verify(blockHeightManager).handleCommitMessage(signedCommit); verify(blockHeightManager).handleRoundChangeMessage(signedRoundChange); @@ -197,7 +194,7 @@ public void proposalForCurrentHeightIsPassedToBlockHeightManager() { ibftController.handleMessageEvent(new IbftReceivedMessageEvent(proposalMessage)); assertThat(futureMessages).isEmpty(); - verify(blockHeightManager).handleProposalMessage(signedProposal, proposalMessage); + verify(blockHeightManager).handleProposalMessage(signedProposal); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/PeerInfo.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/PeerInfo.java index 0fbaac1802..f7bd0daf56 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/PeerInfo.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/PeerInfo.java @@ -14,6 +14,9 @@ import static tech.pegasys.pantheon.util.bytes.BytesValue.wrap; +import tech.pegasys.pantheon.crypto.SECP256K1.PublicKey; +import tech.pegasys.pantheon.ethereum.core.Address; +import tech.pegasys.pantheon.ethereum.core.Util; import tech.pegasys.pantheon.ethereum.rlp.RLPInput; import tech.pegasys.pantheon.ethereum.rlp.RLPOutput; import tech.pegasys.pantheon.util.bytes.BytesValue; @@ -82,6 +85,11 @@ public BytesValue getNodeId() { return nodeId; } + public Address getAddress() { + final PublicKey remotePublicKey = PublicKey.create(nodeId); + return Util.publicKeyToAddress(remotePublicKey); + } + public void writeTo(final RLPOutput out) { out.startList(); out.writeUnsignedByte(getVersion()); From d83aeaa37b896b56b1e35377348c470cb9f33ca2 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 20 Dec 2018 18:10:25 +1000 Subject: [PATCH 05/33] [NC-1909] renamed variables to reflect them being payloads --- .../pantheon/consensus/ibft/IbftGossip.java | 64 +++++++++++++------ .../ibft/network/IbftNetworkPeers.java | 9 +-- .../ibft/statemachine/IbftController.java | 43 +++++++------ 3 files changed, 69 insertions(+), 47 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java index 93720399aa..098bf51a92 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java @@ -1,11 +1,17 @@ +/* + * Copyright 2018 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; -import java.util.Arrays; -import java.util.Collections; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessageData; @@ -20,9 +26,15 @@ import tech.pegasys.pantheon.consensus.ibft.network.IbftNetworkPeers; import tech.pegasys.pantheon.crypto.SECP256K1.Signature; import tech.pegasys.pantheon.ethereum.core.Address; -import tech.pegasys.pantheon.ethereum.p2p.api.Message; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + public class IbftGossip { private final IbftNetworkPeers peers; @@ -39,32 +51,46 @@ public IbftGossip(IbftNetworkPeers peers) { this.peers = peers; } - public boolean gossipProposalPayload(SignedData payload, Address... excludeAddresses) { + public boolean gossipProposalPayload( + SignedData payload, Address... excludeAddresses) { final ProposalMessageData messageData = ProposalMessageData.create(payload); - return gossipMessage(payload.getSignature(), messageData, payload.getSender(), excludeAddresses); + return gossipMessage( + payload.getSignature(), messageData, payload.getSender(), excludeAddresses); } - public boolean gossipPreparePayload(SignedData payload, Address... excludeAddresses) { + public boolean gossipPreparePayload( + SignedData payload, Address... excludeAddresses) { final PrepareMessageData messageData = PrepareMessageData.create(payload); - return gossipMessage(payload.getSignature(), messageData, payload.getSender(), excludeAddresses); + return gossipMessage( + payload.getSignature(), messageData, payload.getSender(), excludeAddresses); } - public boolean gossipCommitPayload(SignedData payload, Address... excludeAddresses) { + public boolean gossipCommitPayload( + SignedData payload, Address... excludeAddresses) { final CommitMessageData messageData = CommitMessageData.create(payload); - return gossipMessage(payload.getSignature(), messageData, payload.getSender(), excludeAddresses); + return gossipMessage( + payload.getSignature(), messageData, payload.getSender(), excludeAddresses); } - public boolean gossipRoundChangePayload(SignedData payload, Address... excludeAddresses) { + public boolean gossipRoundChangePayload( + SignedData payload, Address... excludeAddresses) { final RoundChangeMessageData messageData = RoundChangeMessageData.create(payload); - return gossipMessage(payload.getSignature(), messageData, payload.getSender(), excludeAddresses); + return gossipMessage( + payload.getSignature(), messageData, payload.getSender(), excludeAddresses); } - public boolean gossipNewRoundPayload(SignedData payload, Address... excludeAddresses) { + public boolean gossipNewRoundPayload( + SignedData payload, Address... excludeAddresses) { final NewRoundMessageData messageData = NewRoundMessageData.create(payload); - return gossipMessage(payload.getSignature(), messageData, payload.getSender(), excludeAddresses); + return gossipMessage( + payload.getSignature(), messageData, payload.getSender(), excludeAddresses); } - private boolean gossipMessage(Signature signature, MessageData messageData, Address payloadSigner, Address... excludeAddresses) { + private boolean gossipMessage( + Signature signature, + MessageData messageData, + Address payloadSigner, + Address... excludeAddresses) { if (seenMessages.contains(signature)) { return false; } else { @@ -75,6 +101,4 @@ private boolean gossipMessage(Signature signature, MessageData messageData, Addr return true; } } - - } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java index 645bab960d..9519ceef37 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java @@ -12,17 +12,11 @@ */ package tech.pegasys.pantheon.consensus.ibft.network; -import com.google.common.collect.Lists; -import com.sun.tools.javac.util.List; -import java.util.Arrays; import tech.pegasys.pantheon.consensus.common.ValidatorProvider; -import tech.pegasys.pantheon.crypto.SECP256K1.PublicKey; import tech.pegasys.pantheon.ethereum.core.Address; -import tech.pegasys.pantheon.ethereum.core.Util; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection.PeerNotConnected; -import tech.pegasys.pantheon.util.bytes.BytesValue; import java.util.Collection; import java.util.Map; @@ -60,7 +54,8 @@ public void multicastToValidators(final MessageData message) { sendMessageToSpecificAddresses(validators, message); } - public void multicastToValidatorsExcept(final MessageData message, Collection
exceptAddresses) { + public void multicastToValidatorsExcept( + final MessageData message, Collection
exceptAddresses) { final Collection
validators = validatorProvider.getValidators(); validators.removeIf(exceptAddresses::contains); sendMessageToSpecificAddresses(validators, message); diff --git 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 index 93f810db50..df97dcb47e 100644 --- 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 @@ -88,47 +88,50 @@ private void handleMessage(final Message message) { final MessageData messageData = message.getData(); switch (messageData.getCode()) { case IbftV2.PROPOSAL: - final SignedData proposalMsg = + final SignedData signedProposalPayload = ProposalMessageData.fromMessageData(messageData).decode(); - if (processMessage(proposalMsg, message)) { - currentHeightManager.handleProposalMessage(proposalMsg); - gossiper.gossipProposalPayload(proposalMsg, message.getConnection().getPeer().getAddress()); + if (processMessage(signedProposalPayload, message)) { + currentHeightManager.handleProposalMessage(signedProposalPayload); + gossiper.gossipProposalPayload( + signedProposalPayload, message.getConnection().getPeer().getAddress()); } break; case IbftV2.PREPARE: - final SignedData prepareMsg = + final SignedData signedPreparePayload = PrepareMessageData.fromMessageData(messageData).decode(); - if (processMessage(prepareMsg, message)) { - currentHeightManager.handlePrepareMessage(prepareMsg); - gossiper.gossipPreparePayload(prepareMsg, message.getConnection().getPeer().getAddress()); + if (processMessage(signedPreparePayload, message)) { + currentHeightManager.handlePrepareMessage(signedPreparePayload); + gossiper.gossipPreparePayload(signedPreparePayload, message.getConnection().getPeer().getAddress()); } break; case IbftV2.COMMIT: - final SignedData commitMsg = + final SignedData signedCommitPayload = CommitMessageData.fromMessageData(messageData).decode(); - if (processMessage(commitMsg, message)) { - currentHeightManager.handleCommitMessage(commitMsg); - gossiper.gossipCommitPayload(commitMsg, message.getConnection().getPeer().getAddress()); + if (processMessage(signedCommitPayload, message)) { + currentHeightManager.handleCommitMessage(signedCommitPayload); + gossiper.gossipCommitPayload(signedCommitPayload, message.getConnection().getPeer().getAddress()); } break; case IbftV2.ROUND_CHANGE: - final SignedData roundChangeMsg = + final SignedData signedRoundChangePayload = RoundChangeMessageData.fromMessageData(messageData).decode(); - if (processMessage(roundChangeMsg, message)) { - currentHeightManager.handleRoundChangeMessage(roundChangeMsg); - gossiper.gossipRoundChangePayload(roundChangeMsg, message.getConnection().getPeer().getAddress()); + if (processMessage(signedRoundChangePayload, message)) { + currentHeightManager.handleRoundChangeMessage(signedRoundChangePayload); + gossiper.gossipRoundChangePayload( + signedRoundChangePayload, message.getConnection().getPeer().getAddress()); } break; case IbftV2.NEW_ROUND: - final SignedData newRoundMsg = + final SignedData signedNewRoundPayload = NewRoundMessageData.fromMessageData(messageData).decode(); - if (processMessage(newRoundMsg, message)) { - currentHeightManager.handleNewRoundMessage(newRoundMsg); - gossiper.gossipNewRoundPayload(newRoundMsg, message.getConnection().getPeer().getAddress()); + if (processMessage(signedNewRoundPayload, message)) { + currentHeightManager.handleNewRoundMessage(signedNewRoundPayload); + gossiper.gossipNewRoundPayload( + signedNewRoundPayload, message.getConnection().getPeer().getAddress()); } break; From a83ef6b4ad82908e8be5b376ecca276493777956 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 20 Dec 2018 18:11:08 +1000 Subject: [PATCH 06/33] [NC-1909] round of spotless --- .../consensus/ibft/statemachine/IbftController.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git 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 index df97dcb47e..d7a19ed749 100644 --- 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 @@ -102,7 +102,8 @@ private void handleMessage(final Message message) { PrepareMessageData.fromMessageData(messageData).decode(); if (processMessage(signedPreparePayload, message)) { currentHeightManager.handlePrepareMessage(signedPreparePayload); - gossiper.gossipPreparePayload(signedPreparePayload, message.getConnection().getPeer().getAddress()); + gossiper.gossipPreparePayload( + signedPreparePayload, message.getConnection().getPeer().getAddress()); } break; @@ -111,7 +112,8 @@ private void handleMessage(final Message message) { CommitMessageData.fromMessageData(messageData).decode(); if (processMessage(signedCommitPayload, message)) { currentHeightManager.handleCommitMessage(signedCommitPayload); - gossiper.gossipCommitPayload(signedCommitPayload, message.getConnection().getPeer().getAddress()); + gossiper.gossipCommitPayload( + signedCommitPayload, message.getConnection().getPeer().getAddress()); } break; From 45a230e7d00561ace2ac16c87a2073ad7c620179 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 20 Dec 2018 19:54:25 +1000 Subject: [PATCH 07/33] [NC-1909] tests back to where they were --- .../pantheon/consensus/ibft/IbftGossip.java | 5 +-- .../ibft/network/MockPeerFactory.java | 34 +++++++++++++++++++ .../ibft/statemachine/IbftControllerTest.java | 29 +++++++++------- 3 files changed, 54 insertions(+), 14 deletions(-) create mode 100644 consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/MockPeerFactory.java diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java index 098bf51a92..97dab6763b 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java @@ -28,13 +28,14 @@ import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; -import java.util.Arrays; import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; +import com.google.common.collect.Lists; + public class IbftGossip { private final IbftNetworkPeers peers; @@ -94,7 +95,7 @@ private boolean gossipMessage( if (seenMessages.contains(signature)) { return false; } else { - final List
excludeAddressesList = Arrays.asList(excludeAddresses); + final List
excludeAddressesList = Lists.newArrayList(excludeAddresses); excludeAddressesList.add(payloadSigner); peers.multicastToValidatorsExcept(messageData, excludeAddressesList); seenMessages.add(signature); diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/MockPeerFactory.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/MockPeerFactory.java new file mode 100644 index 0000000000..559271ddd1 --- /dev/null +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/MockPeerFactory.java @@ -0,0 +1,34 @@ +/* + * Copyright 2018 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.network; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; +import tech.pegasys.pantheon.ethereum.p2p.wire.PeerInfo; + +public class MockPeerFactory { + public static PeerConnection create() { + PeerConnection peerConnection = mock(PeerConnection.class); + PeerInfo peerInfo = createPeerInfo(); + when(peerConnection.getPeer()).thenReturn(peerInfo); + return peerConnection; + } + + public static PeerInfo createPeerInfo() { + PeerInfo peerInfo = mock(PeerInfo.class); + when(peerInfo.getAddress()).thenReturn(null); + return peerInfo; + } +} diff --git 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 index 5613568ed3..7266657e51 100644 --- 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 @@ -16,6 +16,7 @@ import static org.assertj.core.util.Lists.newArrayList; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -38,10 +39,13 @@ import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangePayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; +import tech.pegasys.pantheon.consensus.ibft.network.IbftNetworkPeers; +import tech.pegasys.pantheon.consensus.ibft.network.MockPeerFactory; import tech.pegasys.pantheon.ethereum.chain.Blockchain; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.p2p.api.Message; +import tech.pegasys.pantheon.ethereum.p2p.wire.DefaultMessage; import java.util.HashMap; import java.util.List; @@ -64,27 +68,27 @@ public class IbftControllerTest { @Mock private IbftBlockHeightManager blockHeightManager; @Mock private SignedData signedProposal; - @Mock private Message proposalMessage; + private Message proposalMessage; @Mock private ProposalMessageData proposalMessageData; @Mock private ProposalPayload proposalPayload; @Mock private SignedData signedPrepare; - @Mock private Message prepareMessage; + private Message prepareMessage; @Mock private PrepareMessageData prepareMessageData; @Mock private PreparePayload preparePayload; @Mock private SignedData signedCommit; - @Mock private Message commitMessage; + private Message commitMessage; @Mock private CommitMessageData commitMessageData; @Mock private CommitPayload commitPayload; @Mock private SignedData signedNewRound; - @Mock private Message newRoundMessage; + private Message newRoundMessage; @Mock private NewRoundMessageData newRoundMessageData; @Mock private NewRoundPayload newRoundPayload; @Mock private SignedData signedRoundChange; - @Mock private Message roundChangeMessage; + private Message roundChangeMessage; @Mock private RoundChangeMessageData roundChangeMessageData; @Mock private RoundChangePayload roundChangePayload; @@ -97,11 +101,12 @@ public class IbftControllerTest { @Before public void setup() { - ibftController = - new IbftController(blockChain, ibftFinalState, blockHeightManagerFactory, futureMessages); when(blockChain.getChainHeadHeader()).thenReturn(blockHeader); when(blockHeightManagerFactory.create(blockHeader)).thenReturn(blockHeightManager); when(ibftFinalState.getValidators()).thenReturn(ImmutableList.of(validator)); + when(ibftFinalState.getPeers()).thenReturn(mock(IbftNetworkPeers.class)); + ibftController = + new IbftController(blockChain, ibftFinalState, blockHeightManagerFactory, futureMessages); } @Test @@ -408,7 +413,7 @@ private void setupProposal( when(proposalPayload.getRoundIdentifier()).thenReturn(roundIdentifier); when(proposalMessageData.getCode()).thenReturn(IbftV2.PROPOSAL); when(proposalMessageData.decode()).thenReturn(signedProposal); - when(proposalMessage.getData()).thenReturn(proposalMessageData); + proposalMessage = new DefaultMessage(MockPeerFactory.create(), proposalMessageData); } private void setupPrepare( @@ -418,7 +423,7 @@ private void setupPrepare( when(preparePayload.getRoundIdentifier()).thenReturn(roundIdentifier); when(prepareMessageData.getCode()).thenReturn(IbftV2.PREPARE); when(prepareMessageData.decode()).thenReturn(signedPrepare); - when(prepareMessage.getData()).thenReturn(prepareMessageData); + prepareMessage = new DefaultMessage(MockPeerFactory.create(), prepareMessageData); } private void setupCommit( @@ -428,7 +433,7 @@ private void setupCommit( when(commitPayload.getRoundIdentifier()).thenReturn(roundIdentifier); when(commitMessageData.getCode()).thenReturn(IbftV2.COMMIT); when(commitMessageData.decode()).thenReturn(signedCommit); - when(commitMessage.getData()).thenReturn(commitMessageData); + commitMessage = new DefaultMessage(MockPeerFactory.create(), commitMessageData); } private void setupNewRound( @@ -438,7 +443,7 @@ private void setupNewRound( when(newRoundPayload.getRoundIdentifier()).thenReturn(roundIdentifier); when(newRoundMessageData.getCode()).thenReturn(IbftV2.NEW_ROUND); when(newRoundMessageData.decode()).thenReturn(signedNewRound); - when(newRoundMessage.getData()).thenReturn(newRoundMessageData); + newRoundMessage = new DefaultMessage(MockPeerFactory.create(), newRoundMessageData); } private void setupRoundChange( @@ -448,6 +453,6 @@ private void setupRoundChange( when(roundChangePayload.getRoundIdentifier()).thenReturn(roundIdentifier); when(roundChangeMessageData.getCode()).thenReturn(IbftV2.ROUND_CHANGE); when(roundChangeMessageData.decode()).thenReturn(signedRoundChange); - when(roundChangeMessage.getData()).thenReturn(roundChangeMessageData); + roundChangeMessage = new DefaultMessage(MockPeerFactory.create(), roundChangeMessageData); } } From 078228f35fd224d4c58d21f980bb22c23efe0b1c Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Fri, 21 Dec 2018 15:48:04 +1000 Subject: [PATCH 08/33] [NC-1909] added gossiping of messages to the height manager --- .../pantheon/consensus/ibft/IbftGossip.java | 1 + .../ibft/statemachine/IbftController.java | 12 ++++++++--- .../ibft/statemachine/IbftControllerTest.java | 21 +++++++++++++++---- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java index 97dab6763b..e9b75f1f53 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java @@ -40,6 +40,7 @@ public class IbftGossip { private final IbftNetworkPeers peers; private static final int SEEN_MESSAGES_SIZE = 10_000; + // Set that starts evicting members when it hits capacity private final Set seenMessages = Collections.newSetFromMap( new LinkedHashMap() { diff --git 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 index d7a19ed749..97ae4fa5e0 100644 --- 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 @@ -60,7 +60,12 @@ public IbftController( final Blockchain blockchain, final IbftFinalState ibftFinalState, final IbftBlockHeightManagerFactory ibftBlockHeightManagerFactory) { - this(blockchain, ibftFinalState, ibftBlockHeightManagerFactory, Maps.newHashMap()); + this( + blockchain, + ibftFinalState, + ibftBlockHeightManagerFactory, + Maps.newHashMap(), + new IbftGossip(ibftFinalState.getPeers())); } @VisibleForTesting @@ -68,12 +73,13 @@ public IbftController( final Blockchain blockchain, final IbftFinalState ibftFinalState, final IbftBlockHeightManagerFactory ibftBlockHeightManagerFactory, - final Map> futureMessages) { + final Map> futureMessages, + final IbftGossip gossiper) { this.blockchain = blockchain; this.ibftFinalState = ibftFinalState; this.ibftBlockHeightManagerFactory = ibftBlockHeightManagerFactory; this.futureMessages = futureMessages; - this.gossiper = new IbftGossip(ibftFinalState.getPeers()); + this.gossiper = gossiper; } public void start() { diff --git 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 index 7266657e51..afb24f50f6 100644 --- 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 @@ -16,13 +16,13 @@ import static org.assertj.core.util.Lists.newArrayList; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.atLeastOnce; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; +import tech.pegasys.pantheon.consensus.ibft.IbftGossip; import tech.pegasys.pantheon.consensus.ibft.ibftevent.BlockTimerExpiry; import tech.pegasys.pantheon.consensus.ibft.ibftevent.IbftReceivedMessageEvent; import tech.pegasys.pantheon.consensus.ibft.ibftevent.NewChainHead; @@ -39,7 +39,6 @@ import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangePayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; -import tech.pegasys.pantheon.consensus.ibft.network.IbftNetworkPeers; import tech.pegasys.pantheon.consensus.ibft.network.MockPeerFactory; import tech.pegasys.pantheon.ethereum.chain.Blockchain; import tech.pegasys.pantheon.ethereum.core.Address; @@ -97,6 +96,7 @@ public class IbftControllerTest { private final Address unknownValidator = Address.fromHexString("0x2"); private final ConsensusRoundIdentifier futureRoundIdentifier = new ConsensusRoundIdentifier(2, 0); private ConsensusRoundIdentifier roundIdentifier = new ConsensusRoundIdentifier(0, 0); + @Mock private IbftGossip ibftGossip; private IbftController ibftController; @Before @@ -104,9 +104,9 @@ public void setup() { when(blockChain.getChainHeadHeader()).thenReturn(blockHeader); when(blockHeightManagerFactory.create(blockHeader)).thenReturn(blockHeightManager); when(ibftFinalState.getValidators()).thenReturn(ImmutableList.of(validator)); - when(ibftFinalState.getPeers()).thenReturn(mock(IbftNetworkPeers.class)); ibftController = - new IbftController(blockChain, ibftFinalState, blockHeightManagerFactory, futureMessages); + new IbftController( + blockChain, ibftFinalState, blockHeightManagerFactory, futureMessages, ibftGossip); } @Test @@ -140,8 +140,11 @@ public void startsNewBlockHeightManagerAndReplaysFutureMessages() { verify(blockHeightManager).start(); verify(blockHeightManager, never()).handleProposalMessage(signedProposal); verify(blockHeightManager).handlePrepareMessage(signedPrepare); + verify(ibftGossip).gossipPreparePayload(signedPrepare, null); verify(blockHeightManager).handleCommitMessage(signedCommit); + verify(ibftGossip).gossipCommitPayload(signedCommit, null); verify(blockHeightManager).handleRoundChangeMessage(signedRoundChange); + verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, null); verify(blockHeightManager, never()).handleNewRoundMessage(signedNewRound); } @@ -166,10 +169,15 @@ public void createsNewBlockHeightManagerAndReplaysFutureMessagesOnNewChainHeadEv verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verify(blockHeightManager).handleProposalMessage(signedProposal); + verify(ibftGossip).gossipProposalPayload(signedProposal, null); verify(blockHeightManager).handlePrepareMessage(signedPrepare); + verify(ibftGossip).gossipPreparePayload(signedPrepare, null); verify(blockHeightManager).handleCommitMessage(signedCommit); + verify(ibftGossip).gossipCommitPayload(signedCommit, null); verify(blockHeightManager).handleRoundChangeMessage(signedRoundChange); + verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, null); verify(blockHeightManager).handleNewRoundMessage(signedNewRound); + verify(ibftGossip).gossipNewRoundPayload(signedNewRound, null); } @Test @@ -200,6 +208,7 @@ public void proposalForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handleProposalMessage(signedProposal); + verify(ibftGossip).gossipProposalPayload(signedProposal, null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -213,6 +222,7 @@ public void prepareForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handlePrepareMessage(signedPrepare); + verify(ibftGossip).gossipPreparePayload(signedPrepare, null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -226,6 +236,7 @@ public void commitForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handleCommitMessage(signedCommit); + verify(ibftGossip).gossipCommitPayload(signedCommit, null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -240,6 +251,7 @@ public void newRoundForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handleNewRoundMessage(signedNewRound); + verify(ibftGossip).gossipNewRoundPayload(signedNewRound, null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -254,6 +266,7 @@ public void roundChangeForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handleRoundChangeMessage(signedRoundChange); + verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); From 2ed58ba5e71d6000ff0fe12fc594d4022eb3c47b Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 3 Jan 2019 08:00:42 +1000 Subject: [PATCH 09/33] [NC-1909] minor docs --- .../tech/pegasys/pantheon/consensus/ibft/IbftGossip.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java index e9b75f1f53..c33ce11248 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java @@ -36,10 +36,15 @@ import com.google.common.collect.Lists; +/** + * Class responsible for rebroadcasting IBFT messages to known validators + */ public class IbftGossip { private final IbftNetworkPeers peers; + // Size of the seenMessages cache, should end up utilising 65bytes * this number + some meta data private static final int SEEN_MESSAGES_SIZE = 10_000; + // Set that starts evicting members when it hits capacity private final Set seenMessages = Collections.newSetFromMap( From d6b3c97fe1ce7d834291baf7730d7a93bd3f1333 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 3 Jan 2019 08:08:09 +1000 Subject: [PATCH 10/33] [NC-1909] wrong assertions import --- .../java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java | 4 +--- .../consensus/ibft/statemachine/IbftControllerTest.java | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java index c33ce11248..2776cdb3e9 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java @@ -36,9 +36,7 @@ import com.google.common.collect.Lists; -/** - * Class responsible for rebroadcasting IBFT messages to known validators - */ +/** Class responsible for rebroadcasting IBFT messages to known validators */ public class IbftGossip { private final IbftNetworkPeers peers; diff --git 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 index afb24f50f6..1876608984 100644 --- 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 @@ -12,7 +12,7 @@ */ package tech.pegasys.pantheon.consensus.ibft.statemachine; -import static org.assertj.core.api.Java6Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.util.Lists.newArrayList; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.atLeastOnce; From e0f8ee21d5071c327f779c189b045894717a5c02 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 3 Jan 2019 08:15:29 +1000 Subject: [PATCH 11/33] [NC-1909] javadoc for IbftGossip --- .../pantheon/consensus/ibft/IbftGossip.java | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java index 2776cdb3e9..fd689e9953 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java @@ -52,10 +52,23 @@ protected boolean removeEldestEntry(Map.Entry eldest) { } }); + /** + * Constructore that attaches gossip logic to a set of peers + * @param peers The always up to date set of connected peers that understand IBFT + */ public IbftGossip(IbftNetworkPeers peers) { this.peers = peers; } + /** + * Retransmit a given proposal payload to all IBFT nodes + * + * @param payload The signed payload to retransmit + * @param excludeAddresses A list of addresses to not communicate this to. Does not need to + * include the payload signer, that is handled implicitly + * @return Boolean of whether the message was rebroadcast or ignored as it's already been + * broadcast in recent memory + */ public boolean gossipProposalPayload( SignedData payload, Address... excludeAddresses) { final ProposalMessageData messageData = ProposalMessageData.create(payload); @@ -63,6 +76,15 @@ public boolean gossipProposalPayload( payload.getSignature(), messageData, payload.getSender(), excludeAddresses); } + /** + * Retransmit a given prepare payload to all IBFT nodes + * + * @param payload The signed payload to retransmit + * @param excludeAddresses A list of addresses to not communicate this to. Does not need to + * include the payload signer, that is handled implicitly + * @return Boolean of whether the message was rebroadcast or ignored as it's already been + * broadcast in recent memory + */ public boolean gossipPreparePayload( SignedData payload, Address... excludeAddresses) { final PrepareMessageData messageData = PrepareMessageData.create(payload); @@ -70,6 +92,15 @@ public boolean gossipPreparePayload( payload.getSignature(), messageData, payload.getSender(), excludeAddresses); } + /** + * Retransmit a given commit payload to all IBFT nodes + * + * @param payload The signed payload to retransmit + * @param excludeAddresses A list of addresses to not communicate this to. Does not need to + * include the payload signer, that is handled implicitly + * @return Boolean of whether the message was rebroadcast or ignored as it's already been + * broadcast in recent memory + */ public boolean gossipCommitPayload( SignedData payload, Address... excludeAddresses) { final CommitMessageData messageData = CommitMessageData.create(payload); @@ -77,6 +108,15 @@ public boolean gossipCommitPayload( payload.getSignature(), messageData, payload.getSender(), excludeAddresses); } + /** + * Retransmit a given round change payload to all IBFT nodes + * + * @param payload The signed payload to retransmit + * @param excludeAddresses A list of addresses to not communicate this to. Does not need to + * include the payload signer, that is handled implicitly + * @return Boolean of whether the message was rebroadcast or ignored as it's already been + * broadcast in recent memory + */ public boolean gossipRoundChangePayload( SignedData payload, Address... excludeAddresses) { final RoundChangeMessageData messageData = RoundChangeMessageData.create(payload); @@ -84,6 +124,15 @@ public boolean gossipRoundChangePayload( payload.getSignature(), messageData, payload.getSender(), excludeAddresses); } + /** + * Retransmit a given new round payload to all IBFT nodes + * + * @param payload The signed payload to retransmit + * @param excludeAddresses A list of addresses to not communicate this to. Does not need to + * include the payload signer, that is handled implicitly + * @return Boolean of whether the message was rebroadcast or ignored as it's already been + * broadcast in recent memory + */ public boolean gossipNewRoundPayload( SignedData payload, Address... excludeAddresses) { final NewRoundMessageData messageData = NewRoundMessageData.create(payload); @@ -91,6 +140,7 @@ public boolean gossipNewRoundPayload( payload.getSignature(), messageData, payload.getSender(), excludeAddresses); } + // Checks whether a payloads signature has been seen recently, if not it performs the retransmit private boolean gossipMessage( Signature signature, MessageData messageData, From b7bf2bc1c86803a03b3d581919e794b6206b89bf Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 3 Jan 2019 09:18:48 +1000 Subject: [PATCH 12/33] [NC-1909] first couple of tests of the gossiper --- .../pantheon/consensus/ibft/IbftGossip.java | 1 + .../consensus/ibft/IbftGossipTest.java | 107 ++++++++++++++++++ .../p2p/wire/AbstractMessageData.java | 19 ++++ 3 files changed, 127 insertions(+) create mode 100644 consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java index fd689e9953..d6b57ec593 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java @@ -54,6 +54,7 @@ protected boolean removeEldestEntry(Map.Entry eldest) { /** * Constructore that attaches gossip logic to a set of peers + * * @param peers The always up to date set of connected peers that understand IBFT */ public IbftGossip(IbftNetworkPeers peers) { diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java new file mode 100644 index 0000000000..6d331bc8d9 --- /dev/null +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java @@ -0,0 +1,107 @@ +/* + * 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; + +import static com.google.common.collect.Lists.newArrayList; +import static java.util.Collections.singletonList; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.MessageFactory; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparePayload; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; +import tech.pegasys.pantheon.consensus.ibft.network.IbftNetworkPeers; +import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; +import tech.pegasys.pantheon.ethereum.core.AddressHelpers; +import tech.pegasys.pantheon.ethereum.core.Block; +import tech.pegasys.pantheon.ethereum.core.Hash; +import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class IbftGossipTest { + private IbftGossip ibftGossip; + @Mock private IbftNetworkPeers ibftNetworkPeers; + + @Before + public void setup() { + ibftGossip = new IbftGossip(ibftNetworkPeers); + } + + private SignedData createSignedProposalPayload(final KeyPair signerKeys) { + final MessageFactory messageFactory = new MessageFactory(signerKeys); + final ConsensusRoundIdentifier roundIdentifier = + new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); + final Block block = + TestHelpers.createProposalBlock(singletonList(AddressHelpers.ofValue(1)), 0); + return messageFactory.createSignedProposalPayload(roundIdentifier, block); + } + + private SignedData createSignedPreparePayload(final KeyPair signerKeys) { + final MessageFactory messageFactory = new MessageFactory(signerKeys); + final ConsensusRoundIdentifier roundIdentifier = + new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); + return messageFactory.createSignedPreparePayload( + roundIdentifier, Hash.fromHexStringLenient("0")); + } + + @Test + public void rebroadcastsProposalToAllExceptSigner() { + final KeyPair keypair = KeyPair.generate(); + final SignedData payload = createSignedProposalPayload(keypair); + ibftGossip.gossipProposalPayload(payload); + final MessageData messageData = ProposalMessageData.create(payload); + verify(ibftNetworkPeers) + .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + } + + @Test + public void rebroadcastsProposalOnlyOnce() { + final KeyPair keypair = KeyPair.generate(); + final SignedData payload = createSignedProposalPayload(keypair); + ibftGossip.gossipProposalPayload(payload); + ibftGossip.gossipProposalPayload(payload); + final MessageData messageData = ProposalMessageData.create(payload); + verify(ibftNetworkPeers, times(1)) + .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + } + + @Test + public void rebroadcastsPrepareToAllExceptSigner() { + final KeyPair keypair = KeyPair.generate(); + final SignedData payload = createSignedPreparePayload(keypair); + ibftGossip.gossipPreparePayload(payload); + final MessageData messageData = PrepareMessageData.create(payload); + verify(ibftNetworkPeers) + .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + } + + @Test + public void rebroadcastsPrepareOnlyOnce() { + final KeyPair keypair = KeyPair.generate(); + final SignedData payload = createSignedPreparePayload(keypair); + ibftGossip.gossipPreparePayload(payload); + ibftGossip.gossipPreparePayload(payload); + final MessageData messageData = PrepareMessageData.create(payload); + verify(ibftNetworkPeers, times(1)) + .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + } +} diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/AbstractMessageData.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/AbstractMessageData.java index 80da64bced..0ff7a35994 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/AbstractMessageData.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/AbstractMessageData.java @@ -15,6 +15,8 @@ import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; import tech.pegasys.pantheon.util.bytes.BytesValue; +import com.google.common.base.Objects; + public abstract class AbstractMessageData implements MessageData { protected final BytesValue data; @@ -32,4 +34,21 @@ public final int getSize() { public BytesValue getData() { return data; } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + final AbstractMessageData that = (AbstractMessageData) o; + return Objects.equal(data, that.data); + } + + @Override + public int hashCode() { + return Objects.hashCode(data); + } } From 1f2b377955a5745300add53f8e6d04c8ce542228 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 3 Jan 2019 10:19:54 +1000 Subject: [PATCH 13/33] [NC-1909] coverage for ibft gossip --- .../consensus/ibft/IbftGossipTest.java | 207 ++++++++++++++++-- 1 file changed, 183 insertions(+), 24 deletions(-) diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java index 6d331bc8d9..53741b26ad 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java @@ -17,19 +17,35 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessageData; +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.Payload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparePayload; 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.network.IbftNetworkPeers; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; +import tech.pegasys.pantheon.crypto.SECP256K1.Signature; +import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.AddressHelpers; import tech.pegasys.pantheon.ethereum.core.Block; import tech.pegasys.pantheon.ethereum.core.Hash; +import tech.pegasys.pantheon.ethereum.core.Util; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; +import java.math.BigInteger; +import java.util.Optional; +import java.util.function.BiFunction; +import java.util.function.Function; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -63,45 +79,188 @@ private SignedData createSignedPreparePayload(final KeyPair sign roundIdentifier, Hash.fromHexStringLenient("0")); } - @Test - public void rebroadcastsProposalToAllExceptSigner() { + private SignedData createSignedCommitPayload(final KeyPair signerKeys) { + final MessageFactory messageFactory = new MessageFactory(signerKeys); + final ConsensusRoundIdentifier roundIdentifier = + new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); + return messageFactory.createSignedCommitPayload( + roundIdentifier, + Hash.fromHexStringLenient("0"), + Signature.create(BigInteger.ONE, BigInteger.TEN, (byte) 0)); + } + + private SignedData createSignedRoundChangePayload(final KeyPair signerKeys) { + final MessageFactory messageFactory = new MessageFactory(signerKeys); + final ConsensusRoundIdentifier roundIdentifier = + new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); + return messageFactory.createSignedRoundChangePayload(roundIdentifier, Optional.empty()); + } + + private SignedData createSignedNewRoundPayload(final KeyPair signerKeys) { + final MessageFactory messageFactory = new MessageFactory(signerKeys); + final ConsensusRoundIdentifier roundIdentifier = + new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); + final SignedData proposalPayload = createSignedProposalPayload(signerKeys); + return messageFactory.createSignedNewRoundPayload( + roundIdentifier, new RoundChangeCertificate(newArrayList()), proposalPayload); + } + + private

void rebroadcastToAllExceptSigner( + Function> createPayload, + Function, MessageData> createMessageData, + Function, Boolean> gossiper) { final KeyPair keypair = KeyPair.generate(); - final SignedData payload = createSignedProposalPayload(keypair); - ibftGossip.gossipProposalPayload(payload); - final MessageData messageData = ProposalMessageData.create(payload); + final SignedData

payload = createPayload.apply(keypair); + gossiper.apply(payload); + final MessageData messageData = createMessageData.apply(payload); verify(ibftNetworkPeers) .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); } - @Test - public void rebroadcastsProposalOnlyOnce() { + private

void rebroadcastToAllExceptSignerAndSuppliedExceptions( + Function> createPayload, + Function, MessageData> createMessageData, + BiFunction, Address, Boolean> gossiper) { final KeyPair keypair = KeyPair.generate(); - final SignedData payload = createSignedProposalPayload(keypair); - ibftGossip.gossipProposalPayload(payload); - ibftGossip.gossipProposalPayload(payload); - final MessageData messageData = ProposalMessageData.create(payload); + final KeyPair keypair2 = KeyPair.generate(); + final Address key2Address = Util.publicKeyToAddress(keypair2.getPublicKey()); + final SignedData

payload = createPayload.apply(keypair); + gossiper.apply(payload, key2Address); + final MessageData messageData = createMessageData.apply(payload); + verify(ibftNetworkPeers) + .multicastToValidatorsExcept(messageData, newArrayList(key2Address, payload.getSender())); + } + + private

void rebroadcastOnlyOnce( + Function> createPayload, + Function, MessageData> createMessageData, + Function, Boolean> gossiper) { + final KeyPair keypair = KeyPair.generate(); + final SignedData

payload = createPayload.apply(keypair); + gossiper.apply(payload); + gossiper.apply(payload); + final MessageData messageData = createMessageData.apply(payload); verify(ibftNetworkPeers, times(1)) .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); } + @Test + public void rebroadcastsProposalToAllExceptSigner() { + rebroadcastToAllExceptSigner( + this::createSignedProposalPayload, + ProposalMessageData::create, + ibftGossip::gossipProposalPayload); + } + + @Test + public void rebroadcastsProposalToAllExceptSignerAndSuppliedExceptions() { + rebroadcastToAllExceptSignerAndSuppliedExceptions( + this::createSignedProposalPayload, + ProposalMessageData::create, + ibftGossip::gossipProposalPayload); + } + + @Test + public void rebroadcastsProposalOnlyOnce() { + rebroadcastOnlyOnce( + this::createSignedProposalPayload, + ProposalMessageData::create, + ibftGossip::gossipProposalPayload); + } + @Test public void rebroadcastsPrepareToAllExceptSigner() { - final KeyPair keypair = KeyPair.generate(); - final SignedData payload = createSignedPreparePayload(keypair); - ibftGossip.gossipPreparePayload(payload); - final MessageData messageData = PrepareMessageData.create(payload); - verify(ibftNetworkPeers) - .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + rebroadcastToAllExceptSigner( + this::createSignedPreparePayload, + PrepareMessageData::create, + ibftGossip::gossipPreparePayload); + } + + @Test + public void rebroadcastsPrepareToAllExceptSignerAndSuppliedExceptions() { + rebroadcastToAllExceptSignerAndSuppliedExceptions( + this::createSignedPreparePayload, + PrepareMessageData::create, + ibftGossip::gossipPreparePayload); } @Test public void rebroadcastsPrepareOnlyOnce() { - final KeyPair keypair = KeyPair.generate(); - final SignedData payload = createSignedPreparePayload(keypair); - ibftGossip.gossipPreparePayload(payload); - ibftGossip.gossipPreparePayload(payload); - final MessageData messageData = PrepareMessageData.create(payload); - verify(ibftNetworkPeers, times(1)) - .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + rebroadcastOnlyOnce( + this::createSignedPreparePayload, + PrepareMessageData::create, + ibftGossip::gossipPreparePayload); + } + + @Test + public void rebroadcastsCommitToAllExceptSigner() { + rebroadcastToAllExceptSigner( + this::createSignedCommitPayload, + CommitMessageData::create, + ibftGossip::gossipCommitPayload); + } + + @Test + public void rebroadcastsCommitToAllExceptSignerAndSuppliedExceptions() { + rebroadcastToAllExceptSignerAndSuppliedExceptions( + this::createSignedCommitPayload, + CommitMessageData::create, + ibftGossip::gossipCommitPayload); + } + + @Test + public void rebroadcastsCommitOnlyOnce() { + rebroadcastOnlyOnce( + this::createSignedCommitPayload, + CommitMessageData::create, + ibftGossip::gossipCommitPayload); + } + + @Test + public void rebroadcastsRoundChangeToAllExceptSigner() { + rebroadcastToAllExceptSigner( + this::createSignedRoundChangePayload, + RoundChangeMessageData::create, + ibftGossip::gossipRoundChangePayload); + } + + @Test + public void rebroadcastsRoundChangeToAllExceptSignerAndSuppliedExceptions() { + rebroadcastToAllExceptSignerAndSuppliedExceptions( + this::createSignedRoundChangePayload, + RoundChangeMessageData::create, + ibftGossip::gossipRoundChangePayload); + } + + @Test + public void rebroadcastsRoundChangeOnlyOnce() { + rebroadcastOnlyOnce( + this::createSignedRoundChangePayload, + RoundChangeMessageData::create, + ibftGossip::gossipRoundChangePayload); + } + + @Test + public void rebroadcastsNewRoundToAllExceptSigner() { + rebroadcastToAllExceptSigner( + this::createSignedNewRoundPayload, + NewRoundMessageData::create, + ibftGossip::gossipNewRoundPayload); + } + + @Test + public void rebroadcastsNewRoundToAllExceptSignerAndSuppliedExceptions() { + rebroadcastToAllExceptSignerAndSuppliedExceptions( + this::createSignedNewRoundPayload, + NewRoundMessageData::create, + ibftGossip::gossipNewRoundPayload); + } + + @Test + public void rebroadcastsNewRoundOnlyOnce() { + rebroadcastOnlyOnce( + this::createSignedNewRoundPayload, + NewRoundMessageData::create, + ibftGossip::gossipNewRoundPayload); } } From 84d980d394397652d402365e6741a8a79b3f00e4 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 3 Jan 2019 10:32:01 +1000 Subject: [PATCH 14/33] [NC-1909] Test for new broadcast method --- .../ibft/network/IbftNetworkPeersTest.java | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java index 3609bdc829..f07a2a3502 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java @@ -12,6 +12,7 @@ */ package tech.pegasys.pantheon.consensus.ibft.network; +import static com.google.common.collect.Lists.newArrayList; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -42,21 +43,22 @@ @RunWith(MockitoJUnitRunner.class) public class IbftNetworkPeersTest { - private final List

validators = Lists.newArrayList(); - private final List publicKeys = Lists.newArrayList(); + private final List
validators = newArrayList(); + private final List publicKeys = newArrayList(); - private final List peerConnections = Lists.newArrayList(); + private final List peerConnections = newArrayList(); @Before public void setup() { for (int i = 0; i < 4; i++) { final PublicKey pubKey = PublicKey.create(BigInteger.valueOf(i)); publicKeys.add(pubKey); + final Address address = Util.publicKeyToAddress(pubKey); final PeerInfo peerInfo = mock(PeerInfo.class); final PeerConnection peerConnection = mock(PeerConnection.class); when(peerConnection.getPeer()).thenReturn(peerInfo); - when(peerInfo.getNodeId()).thenReturn(pubKey.getEncodedBytes()); + when(peerInfo.getAddress()).thenReturn(address); peerConnections.add(peerConnection); } @@ -93,7 +95,7 @@ public void doesntSendToValidatorsWhichAreNotDirectlyConnected() throws PeerNotC final IbftNetworkPeers peers = new IbftNetworkPeers(validatorProvider); // only add peer connections 1, 2 & 3, none of which should be invoked. - Lists.newArrayList(1, 2, 3).forEach(i -> peers.peerAdded(peerConnections.get(i))); + newArrayList(1, 2, 3).forEach(i -> peers.peerAdded(peerConnections.get(i))); final MessageData messageToSend = new RawMessage(1, BytesValue.EMPTY); peers.multicastToValidators(messageToSend); @@ -103,4 +105,26 @@ public void doesntSendToValidatorsWhichAreNotDirectlyConnected() throws PeerNotC verify(peerConnections.get(2), never()).sendForProtocol(any(), any()); verify(peerConnections.get(3), never()).sendForProtocol(any(), any()); } + + @Test + public void onlyValidatorsAreSentAMessageNotInExcludes() throws PeerNotConnected { + // Only add the first Peer's address to the validators. + final Address validatorAddress = Util.publicKeyToAddress(publicKeys.get(0)); + validators.add(validatorAddress); + final ValidatorProvider validatorProvider = mock(ValidatorProvider.class); + when(validatorProvider.getValidators()).thenReturn(validators); + + final IbftNetworkPeers peers = new IbftNetworkPeers(validatorProvider); + for (final PeerConnection peer : peerConnections) { + peers.peerAdded(peer); + } + + final MessageData messageToSend = new RawMessage(1, BytesValue.EMPTY); + peers.multicastToValidatorsExcept(messageToSend, newArrayList(validatorAddress)); + + verify(peerConnections.get(0), never()).sendForProtocol(any(), any()); + verify(peerConnections.get(1), never()).sendForProtocol(any(), any()); + verify(peerConnections.get(2), never()).sendForProtocol(any(), any()); + verify(peerConnections.get(3), never()).sendForProtocol(any(), any()); + } } From 93b62701fa61b10bfd9fc91ccdf67e932113b499 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 3 Jan 2019 10:44:58 +1000 Subject: [PATCH 15/33] [NC-1909] fixing errorprone checks --- .../pantheon/consensus/ibft/IbftGossip.java | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java index d6b57ec593..7036d7066a 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java @@ -47,7 +47,8 @@ public class IbftGossip { private final Set seenMessages = Collections.newSetFromMap( new LinkedHashMap() { - protected boolean removeEldestEntry(Map.Entry eldest) { + @Override + protected boolean removeEldestEntry(final Map.Entry eldest) { return size() > SEEN_MESSAGES_SIZE; } }); @@ -57,7 +58,7 @@ protected boolean removeEldestEntry(Map.Entry eldest) { * * @param peers The always up to date set of connected peers that understand IBFT */ - public IbftGossip(IbftNetworkPeers peers) { + public IbftGossip(final IbftNetworkPeers peers) { this.peers = peers; } @@ -71,7 +72,7 @@ public IbftGossip(IbftNetworkPeers peers) { * broadcast in recent memory */ public boolean gossipProposalPayload( - SignedData payload, Address... excludeAddresses) { + final SignedData payload, final Address... excludeAddresses) { final ProposalMessageData messageData = ProposalMessageData.create(payload); return gossipMessage( payload.getSignature(), messageData, payload.getSender(), excludeAddresses); @@ -87,7 +88,7 @@ public boolean gossipProposalPayload( * broadcast in recent memory */ public boolean gossipPreparePayload( - SignedData payload, Address... excludeAddresses) { + final SignedData payload, final Address... excludeAddresses) { final PrepareMessageData messageData = PrepareMessageData.create(payload); return gossipMessage( payload.getSignature(), messageData, payload.getSender(), excludeAddresses); @@ -103,7 +104,7 @@ public boolean gossipPreparePayload( * broadcast in recent memory */ public boolean gossipCommitPayload( - SignedData payload, Address... excludeAddresses) { + final SignedData payload, final Address... excludeAddresses) { final CommitMessageData messageData = CommitMessageData.create(payload); return gossipMessage( payload.getSignature(), messageData, payload.getSender(), excludeAddresses); @@ -119,7 +120,7 @@ public boolean gossipCommitPayload( * broadcast in recent memory */ public boolean gossipRoundChangePayload( - SignedData payload, Address... excludeAddresses) { + final SignedData payload, final Address... excludeAddresses) { final RoundChangeMessageData messageData = RoundChangeMessageData.create(payload); return gossipMessage( payload.getSignature(), messageData, payload.getSender(), excludeAddresses); @@ -135,7 +136,7 @@ public boolean gossipRoundChangePayload( * broadcast in recent memory */ public boolean gossipNewRoundPayload( - SignedData payload, Address... excludeAddresses) { + final SignedData payload, final Address... excludeAddresses) { final NewRoundMessageData messageData = NewRoundMessageData.create(payload); return gossipMessage( payload.getSignature(), messageData, payload.getSender(), excludeAddresses); @@ -143,10 +144,10 @@ public boolean gossipNewRoundPayload( // Checks whether a payloads signature has been seen recently, if not it performs the retransmit private boolean gossipMessage( - Signature signature, - MessageData messageData, - Address payloadSigner, - Address... excludeAddresses) { + final Signature signature, + final MessageData messageData, + final Address payloadSigner, + final Address... excludeAddresses) { if (seenMessages.contains(signature)) { return false; } else { From 73fa0ad42f2ba25a157f5a00d277b07da453d86d Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 3 Jan 2019 10:50:40 +1000 Subject: [PATCH 16/33] [NC-1909] another final method param --- .../pantheon/consensus/ibft/network/IbftNetworkPeers.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java index 9519ceef37..2e2c9f91c1 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java @@ -55,7 +55,7 @@ public void multicastToValidators(final MessageData message) { } public void multicastToValidatorsExcept( - final MessageData message, Collection
exceptAddresses) { + final MessageData message, final Collection
exceptAddresses) { final Collection
validators = validatorProvider.getValidators(); validators.removeIf(exceptAddresses::contains); sendMessageToSpecificAddresses(validators, message); From 94749701fcc3e568b66da134d439af1abb285d16 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 3 Jan 2019 10:51:38 +1000 Subject: [PATCH 17/33] [NC-1909] More final method params --- .../consensus/ibft/IbftGossipTest.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java index 53741b26ad..ec37186399 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java @@ -106,9 +106,9 @@ private SignedData createSignedNewRoundPayload(final KeyPair si } private

void rebroadcastToAllExceptSigner( - Function> createPayload, - Function, MessageData> createMessageData, - Function, Boolean> gossiper) { + final Function> createPayload, + final Function, MessageData> createMessageData, + final Function, Boolean> gossiper) { final KeyPair keypair = KeyPair.generate(); final SignedData

payload = createPayload.apply(keypair); gossiper.apply(payload); @@ -118,9 +118,9 @@ private

void rebroadcastToAllExceptSigner( } private

void rebroadcastToAllExceptSignerAndSuppliedExceptions( - Function> createPayload, - Function, MessageData> createMessageData, - BiFunction, Address, Boolean> gossiper) { + final Function> createPayload, + final Function, MessageData> createMessageData, + final BiFunction, Address, Boolean> gossiper) { final KeyPair keypair = KeyPair.generate(); final KeyPair keypair2 = KeyPair.generate(); final Address key2Address = Util.publicKeyToAddress(keypair2.getPublicKey()); @@ -132,9 +132,9 @@ private

void rebroadcastToAllExceptSignerAndSuppliedExceptio } private

void rebroadcastOnlyOnce( - Function> createPayload, - Function, MessageData> createMessageData, - Function, Boolean> gossiper) { + final Function> createPayload, + final Function, MessageData> createMessageData, + final Function, Boolean> gossiper) { final KeyPair keypair = KeyPair.generate(); final SignedData

payload = createPayload.apply(keypair); gossiper.apply(payload); From f1a8d9f09e366a4c3d469b1d86ffacb4bb1a88cf Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 3 Jan 2019 11:02:12 +1000 Subject: [PATCH 18/33] [NC-1909] casting to silence mocking warnings --- .../ibft/network/IbftNetworkPeersTest.java | 1 - .../ibft/statemachine/IbftControllerTest.java | 26 +++++++++---------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java index f07a2a3502..7c40d7b999 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java @@ -34,7 +34,6 @@ import java.math.BigInteger; import java.util.List; -import com.google.common.collect.Lists; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; diff --git 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 index 1876608984..760b03d315 100644 --- 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 @@ -140,11 +140,11 @@ public void startsNewBlockHeightManagerAndReplaysFutureMessages() { verify(blockHeightManager).start(); verify(blockHeightManager, never()).handleProposalMessage(signedProposal); verify(blockHeightManager).handlePrepareMessage(signedPrepare); - verify(ibftGossip).gossipPreparePayload(signedPrepare, null); + verify(ibftGossip).gossipPreparePayload(signedPrepare, (Address) null); verify(blockHeightManager).handleCommitMessage(signedCommit); - verify(ibftGossip).gossipCommitPayload(signedCommit, null); + verify(ibftGossip).gossipCommitPayload(signedCommit, (Address) null); verify(blockHeightManager).handleRoundChangeMessage(signedRoundChange); - verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, null); + verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, (Address) null); verify(blockHeightManager, never()).handleNewRoundMessage(signedNewRound); } @@ -169,15 +169,15 @@ public void createsNewBlockHeightManagerAndReplaysFutureMessagesOnNewChainHeadEv verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verify(blockHeightManager).handleProposalMessage(signedProposal); - verify(ibftGossip).gossipProposalPayload(signedProposal, null); + verify(ibftGossip).gossipProposalPayload(signedProposal, (Address) null); verify(blockHeightManager).handlePrepareMessage(signedPrepare); - verify(ibftGossip).gossipPreparePayload(signedPrepare, null); + verify(ibftGossip).gossipPreparePayload(signedPrepare, (Address) null); verify(blockHeightManager).handleCommitMessage(signedCommit); - verify(ibftGossip).gossipCommitPayload(signedCommit, null); + verify(ibftGossip).gossipCommitPayload(signedCommit, (Address) null); verify(blockHeightManager).handleRoundChangeMessage(signedRoundChange); - verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, null); + verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, (Address) null); verify(blockHeightManager).handleNewRoundMessage(signedNewRound); - verify(ibftGossip).gossipNewRoundPayload(signedNewRound, null); + verify(ibftGossip).gossipNewRoundPayload(signedNewRound, (Address) null); } @Test @@ -208,7 +208,7 @@ public void proposalForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handleProposalMessage(signedProposal); - verify(ibftGossip).gossipProposalPayload(signedProposal, null); + verify(ibftGossip).gossipProposalPayload(signedProposal, (Address) null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -222,7 +222,7 @@ public void prepareForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handlePrepareMessage(signedPrepare); - verify(ibftGossip).gossipPreparePayload(signedPrepare, null); + verify(ibftGossip).gossipPreparePayload(signedPrepare, (Address) null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -236,7 +236,7 @@ public void commitForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handleCommitMessage(signedCommit); - verify(ibftGossip).gossipCommitPayload(signedCommit, null); + verify(ibftGossip).gossipCommitPayload(signedCommit, (Address) null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -251,7 +251,7 @@ public void newRoundForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handleNewRoundMessage(signedNewRound); - verify(ibftGossip).gossipNewRoundPayload(signedNewRound, null); + verify(ibftGossip).gossipNewRoundPayload(signedNewRound, (Address) null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -266,7 +266,7 @@ public void roundChangeForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handleRoundChangeMessage(signedRoundChange); - verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, null); + verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, (Address) null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); From cb6e263f4058e2e22fed10139e96ffd8e4cf3ed2 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 3 Jan 2019 11:11:37 +1000 Subject: [PATCH 19/33] [NC-1909] Making sure that returned values are consumed --- .../pantheon/consensus/ibft/IbftGossipTest.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java index ec37186399..ffc4c872f9 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java @@ -14,6 +14,7 @@ import static com.google.common.collect.Lists.newArrayList; import static java.util.Collections.singletonList; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -111,7 +112,8 @@ private

void rebroadcastToAllExceptSigner( final Function, Boolean> gossiper) { final KeyPair keypair = KeyPair.generate(); final SignedData

payload = createPayload.apply(keypair); - gossiper.apply(payload); + final boolean gossipResult = gossiper.apply(payload); + assertThat(gossipResult).isTrue(); final MessageData messageData = createMessageData.apply(payload); verify(ibftNetworkPeers) .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); @@ -125,7 +127,8 @@ private

void rebroadcastToAllExceptSignerAndSuppliedExceptio final KeyPair keypair2 = KeyPair.generate(); final Address key2Address = Util.publicKeyToAddress(keypair2.getPublicKey()); final SignedData

payload = createPayload.apply(keypair); - gossiper.apply(payload, key2Address); + final boolean gossipResult = gossiper.apply(payload, key2Address); + assertThat(gossipResult).isTrue(); final MessageData messageData = createMessageData.apply(payload); verify(ibftNetworkPeers) .multicastToValidatorsExcept(messageData, newArrayList(key2Address, payload.getSender())); @@ -137,8 +140,10 @@ private

void rebroadcastOnlyOnce( final Function, Boolean> gossiper) { final KeyPair keypair = KeyPair.generate(); final SignedData

payload = createPayload.apply(keypair); - gossiper.apply(payload); - gossiper.apply(payload); + final boolean gossip1Result = gossiper.apply(payload); + final boolean gossip2Result = gossiper.apply(payload); + assertThat(gossip1Result).isTrue(); + assertThat(gossip2Result).isFalse(); final MessageData messageData = createMessageData.apply(payload); verify(ibftNetworkPeers, times(1)) .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); From 17e8a358add96ad9168e7fdaf52f23d69db8ceaf Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 3 Jan 2019 11:25:24 +1000 Subject: [PATCH 20/33] [NC-1909] fixing reworded error message tests --- .../ibftmessage/AbstractIbftMessageData.java | 2 +- .../ibft/ibftmessage/CommitMessageTest.java | 2 +- .../ibft/ibftmessage/NewRoundMessageTest.java | 2 +- .../ibft/ibftmessage/PrepareMessageTest.java | 2 +- ...MessageTest.java => ProposalMessageTest.java} | 16 ++++++++-------- .../ibft/ibftmessage/RoundChangeMessageTest.java | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) rename consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/{PrePrepareMessageTest.java => ProposalMessageTest.java} (79%) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java index 21074dcefd..c019766603 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/AbstractIbftMessageData.java @@ -41,7 +41,7 @@ protected static T fromMessageData( if (code != messageCode) { throw new IllegalArgumentException( String.format( - "MessageData has code %d and thus is not a %s", code, clazz.getSimpleName())); + "MessageData has code %d and thus is not a %s", code, clazz.getSimpleName())); } return constructor.apply(messageData.getData()); diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageTest.java index d9b7f16832..7471b2d3d6 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/CommitMessageTest.java @@ -65,6 +65,6 @@ public void createMessageFailsWhenIncorrectMessageCode() { when(messageData.getCode()).thenReturn(42); assertThatThrownBy(() -> CommitMessageData.fromMessageData(messageData)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Message has code 42 and thus is not a CommitMessageData"); + .hasMessageContaining("MessageData has code 42 and thus is not a CommitMessageData"); } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageTest.java index 3b2ee33c53..fe1511ff5e 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/NewRoundMessageTest.java @@ -65,6 +65,6 @@ public void createMessageFailsWhenIncorrectMessageCode() { when(messageData.getCode()).thenReturn(42); assertThatThrownBy(() -> NewRoundMessageData.fromMessageData(messageData)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Message has code 42 and thus is not a NewRoundMessageData"); + .hasMessageContaining("MessageData has code 42 and thus is not a NewRoundMessageData"); } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageTest.java index 71a010fab7..363c19d14e 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrepareMessageTest.java @@ -65,6 +65,6 @@ public void createMessageFailsWhenIncorrectMessageCode() { when(messageData.getCode()).thenReturn(42); assertThatThrownBy(() -> PrepareMessageData.fromMessageData(messageData)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Message has code 42 and thus is not a PrepareMessageData"); + .hasMessageContaining("MessageData has code 42 and thus is not a PrepareMessageData"); } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrePrepareMessageTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/ProposalMessageTest.java similarity index 79% rename from consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrePrepareMessageTest.java rename to consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/ProposalMessageTest.java index 5a1bdf630c..9480e2bfe7 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/PrePrepareMessageTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/ProposalMessageTest.java @@ -28,25 +28,25 @@ import org.mockito.junit.MockitoJUnitRunner; @RunWith(MockitoJUnitRunner.class) -public class PrePrepareMessageTest { - @Mock private SignedData prePrepareMessageData; +public class ProposalMessageTest { + @Mock private SignedData proposalMessageData; @Mock private BytesValue messageBytes; @Mock private MessageData messageData; @Mock private ProposalMessageData proposalMessage; @Test public void createMessageFromPrePrepareMessageData() { - when(prePrepareMessageData.encode()).thenReturn(messageBytes); - ProposalMessageData proposalMessage = ProposalMessageData.create(prePrepareMessageData); + when(proposalMessageData.encode()).thenReturn(messageBytes); + final ProposalMessageData proposalMessage = ProposalMessageData.create(proposalMessageData); assertThat(proposalMessage.getData()).isEqualTo(messageBytes); assertThat(proposalMessage.getCode()).isEqualTo(IbftV2.PROPOSAL); - verify(prePrepareMessageData).encode(); + verify(proposalMessageData).encode(); } @Test public void createMessageFromPrePrepareMessage() { - ProposalMessageData message = ProposalMessageData.fromMessageData(proposalMessage); + final ProposalMessageData message = ProposalMessageData.fromMessageData(proposalMessage); assertThat(message).isSameAs(proposalMessage); } @@ -54,7 +54,7 @@ public void createMessageFromPrePrepareMessage() { public void createMessageFromGenericMessageData() { when(messageData.getCode()).thenReturn(IbftV2.PROPOSAL); when(messageData.getData()).thenReturn(messageBytes); - ProposalMessageData proposalMessage = ProposalMessageData.fromMessageData(messageData); + final ProposalMessageData proposalMessage = ProposalMessageData.fromMessageData(messageData); assertThat(proposalMessage.getData()).isEqualTo(messageData.getData()); assertThat(proposalMessage.getCode()).isEqualTo(IbftV2.PROPOSAL); @@ -65,6 +65,6 @@ public void createMessageFailsWhenIncorrectMessageCode() { when(messageData.getCode()).thenReturn(42); assertThatThrownBy(() -> ProposalMessageData.fromMessageData(messageData)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Message has code 42 and thus is not a ProposalMessageData"); + .hasMessageContaining("MessageData has code 42 and thus is not a ProposalMessageData"); } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageTest.java index b32219809a..59686579bb 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessage/RoundChangeMessageTest.java @@ -65,6 +65,6 @@ public void createMessageFailsWhenIncorrectMessageCode() { when(messageData.getCode()).thenReturn(42); assertThatThrownBy(() -> RoundChangeMessageData.fromMessageData(messageData)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Message has code 42 and thus is not a RoundChangeMessageData"); + .hasMessageContaining("MessageData has code 42 and thus is not a RoundChangeMessageData"); } } From 39819660991c0652650fc00e91ca4c720f14d517 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Fri, 4 Jan 2019 14:10:00 +1000 Subject: [PATCH 21/33] [NC-1909] fixes from PR comments --- .../pantheon/consensus/ibft/IbftGossip.java | 2 +- .../ibft/statemachine/IbftController.java | 10 +- .../consensus/ibft/IbftGossipTest.java | 152 ++++++------------ .../pantheon/consensus/ibft/TestHelpers.java | 59 +++++++ 4 files changed, 113 insertions(+), 110 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java index 7036d7066a..4a6790ba98 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java @@ -54,7 +54,7 @@ protected boolean removeEldestEntry(final Map.Entry eldest) }); /** - * Constructore that attaches gossip logic to a set of peers + * Constructor that attaches gossip logic to a set of peers * * @param peers The always up to date set of connected peers that understand IBFT */ diff --git 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 index 97ae4fa5e0..df440ba1b7 100644 --- 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 @@ -97,9 +97,9 @@ private void handleMessage(final Message message) { final SignedData signedProposalPayload = ProposalMessageData.fromMessageData(messageData).decode(); if (processMessage(signedProposalPayload, message)) { - currentHeightManager.handleProposalMessage(signedProposalPayload); gossiper.gossipProposalPayload( signedProposalPayload, message.getConnection().getPeer().getAddress()); + currentHeightManager.handleProposalMessage(signedProposalPayload); } break; @@ -107,9 +107,9 @@ private void handleMessage(final Message message) { final SignedData signedPreparePayload = PrepareMessageData.fromMessageData(messageData).decode(); if (processMessage(signedPreparePayload, message)) { - currentHeightManager.handlePrepareMessage(signedPreparePayload); gossiper.gossipPreparePayload( signedPreparePayload, message.getConnection().getPeer().getAddress()); + currentHeightManager.handlePrepareMessage(signedPreparePayload); } break; @@ -117,9 +117,9 @@ private void handleMessage(final Message message) { final SignedData signedCommitPayload = CommitMessageData.fromMessageData(messageData).decode(); if (processMessage(signedCommitPayload, message)) { - currentHeightManager.handleCommitMessage(signedCommitPayload); gossiper.gossipCommitPayload( signedCommitPayload, message.getConnection().getPeer().getAddress()); + currentHeightManager.handleCommitMessage(signedCommitPayload); } break; @@ -127,9 +127,9 @@ private void handleMessage(final Message message) { final SignedData signedRoundChangePayload = RoundChangeMessageData.fromMessageData(messageData).decode(); if (processMessage(signedRoundChangePayload, message)) { - currentHeightManager.handleRoundChangeMessage(signedRoundChangePayload); gossiper.gossipRoundChangePayload( signedRoundChangePayload, message.getConnection().getPeer().getAddress()); + currentHeightManager.handleRoundChangeMessage(signedRoundChangePayload); } break; @@ -137,9 +137,9 @@ private void handleMessage(final Message message) { final SignedData signedNewRoundPayload = NewRoundMessageData.fromMessageData(messageData).decode(); if (processMessage(signedNewRoundPayload, message)) { - currentHeightManager.handleNewRoundMessage(signedNewRoundPayload); gossiper.gossipNewRoundPayload( signedNewRoundPayload, message.getConnection().getPeer().getAddress()); + currentHeightManager.handleNewRoundMessage(signedNewRoundPayload); } break; diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java index ffc4c872f9..bd0f1bc373 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java @@ -13,7 +13,6 @@ package tech.pegasys.pantheon.consensus.ibft; import static com.google.common.collect.Lists.newArrayList; -import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -23,27 +22,14 @@ import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessageData; -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.Payload; -import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparePayload; -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.network.IbftNetworkPeers; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; -import tech.pegasys.pantheon.crypto.SECP256K1.Signature; import tech.pegasys.pantheon.ethereum.core.Address; -import tech.pegasys.pantheon.ethereum.core.AddressHelpers; -import tech.pegasys.pantheon.ethereum.core.Block; -import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.core.Util; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; -import java.math.BigInteger; -import java.util.Optional; import java.util.function.BiFunction; import java.util.function.Function; @@ -63,50 +49,8 @@ public void setup() { ibftGossip = new IbftGossip(ibftNetworkPeers); } - private SignedData createSignedProposalPayload(final KeyPair signerKeys) { - final MessageFactory messageFactory = new MessageFactory(signerKeys); - final ConsensusRoundIdentifier roundIdentifier = - new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); - final Block block = - TestHelpers.createProposalBlock(singletonList(AddressHelpers.ofValue(1)), 0); - return messageFactory.createSignedProposalPayload(roundIdentifier, block); - } - - private SignedData createSignedPreparePayload(final KeyPair signerKeys) { - final MessageFactory messageFactory = new MessageFactory(signerKeys); - final ConsensusRoundIdentifier roundIdentifier = - new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); - return messageFactory.createSignedPreparePayload( - roundIdentifier, Hash.fromHexStringLenient("0")); - } - - private SignedData createSignedCommitPayload(final KeyPair signerKeys) { - final MessageFactory messageFactory = new MessageFactory(signerKeys); - final ConsensusRoundIdentifier roundIdentifier = - new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); - return messageFactory.createSignedCommitPayload( - roundIdentifier, - Hash.fromHexStringLenient("0"), - Signature.create(BigInteger.ONE, BigInteger.TEN, (byte) 0)); - } - - private SignedData createSignedRoundChangePayload(final KeyPair signerKeys) { - final MessageFactory messageFactory = new MessageFactory(signerKeys); - final ConsensusRoundIdentifier roundIdentifier = - new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); - return messageFactory.createSignedRoundChangePayload(roundIdentifier, Optional.empty()); - } - - private SignedData createSignedNewRoundPayload(final KeyPair signerKeys) { - final MessageFactory messageFactory = new MessageFactory(signerKeys); - final ConsensusRoundIdentifier roundIdentifier = - new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); - final SignedData proposalPayload = createSignedProposalPayload(signerKeys); - return messageFactory.createSignedNewRoundPayload( - roundIdentifier, new RoundChangeCertificate(newArrayList()), proposalPayload); - } - private

void rebroadcastToAllExceptSigner( + private

void assertRebroadcastToAllExceptSigner( final Function> createPayload, final Function, MessageData> createMessageData, final Function, Boolean> gossiper) { @@ -119,7 +63,7 @@ private

void rebroadcastToAllExceptSigner( .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); } - private

void rebroadcastToAllExceptSignerAndSuppliedExceptions( + private

void assertRebroadcastToAllExceptSignerAndSuppliedExceptions( final Function> createPayload, final Function, MessageData> createMessageData, final BiFunction, Address, Boolean> gossiper) { @@ -134,7 +78,7 @@ private

void rebroadcastToAllExceptSignerAndSuppliedExceptio .multicastToValidatorsExcept(messageData, newArrayList(key2Address, payload.getSender())); } - private

void rebroadcastOnlyOnce( + private

void assertRebroadcastOnlyOnce( final Function> createPayload, final Function, MessageData> createMessageData, final Function, Boolean> gossiper) { @@ -150,121 +94,121 @@ private

void rebroadcastOnlyOnce( } @Test - public void rebroadcastsProposalToAllExceptSigner() { - rebroadcastToAllExceptSigner( - this::createSignedProposalPayload, + public void assertRebroadcastsProposalToAllExceptSigner() { + assertRebroadcastToAllExceptSigner( + TestHelpers::createSignedProposalPayload, ProposalMessageData::create, ibftGossip::gossipProposalPayload); } @Test - public void rebroadcastsProposalToAllExceptSignerAndSuppliedExceptions() { - rebroadcastToAllExceptSignerAndSuppliedExceptions( - this::createSignedProposalPayload, + public void assertRebroadcastsProposalToAllExceptSignerAndSuppliedExceptions() { + assertRebroadcastToAllExceptSignerAndSuppliedExceptions( + TestHelpers::createSignedProposalPayload, ProposalMessageData::create, ibftGossip::gossipProposalPayload); } @Test - public void rebroadcastsProposalOnlyOnce() { - rebroadcastOnlyOnce( - this::createSignedProposalPayload, + public void assertRebroadcastsProposalOnlyOnce() { + assertRebroadcastOnlyOnce( + TestHelpers::createSignedProposalPayload, ProposalMessageData::create, ibftGossip::gossipProposalPayload); } @Test - public void rebroadcastsPrepareToAllExceptSigner() { - rebroadcastToAllExceptSigner( - this::createSignedPreparePayload, + public void assertRebroadcastsPrepareToAllExceptSigner() { + assertRebroadcastToAllExceptSigner( + TestHelpers::createSignedPreparePayload, PrepareMessageData::create, ibftGossip::gossipPreparePayload); } @Test - public void rebroadcastsPrepareToAllExceptSignerAndSuppliedExceptions() { - rebroadcastToAllExceptSignerAndSuppliedExceptions( - this::createSignedPreparePayload, + public void assertRebroadcastsPrepareToAllExceptSignerAndSuppliedExceptions() { + assertRebroadcastToAllExceptSignerAndSuppliedExceptions( + TestHelpers::createSignedPreparePayload, PrepareMessageData::create, ibftGossip::gossipPreparePayload); } @Test - public void rebroadcastsPrepareOnlyOnce() { - rebroadcastOnlyOnce( - this::createSignedPreparePayload, + public void assertRebroadcastsPrepareOnlyOnce() { + assertRebroadcastOnlyOnce( + TestHelpers::createSignedPreparePayload, PrepareMessageData::create, ibftGossip::gossipPreparePayload); } @Test - public void rebroadcastsCommitToAllExceptSigner() { - rebroadcastToAllExceptSigner( - this::createSignedCommitPayload, + public void assertRebroadcastsCommitToAllExceptSigner() { + assertRebroadcastToAllExceptSigner( + TestHelpers::createSignedCommitPayload, CommitMessageData::create, ibftGossip::gossipCommitPayload); } @Test - public void rebroadcastsCommitToAllExceptSignerAndSuppliedExceptions() { - rebroadcastToAllExceptSignerAndSuppliedExceptions( - this::createSignedCommitPayload, + public void assertRebroadcastsCommitToAllExceptSignerAndSuppliedExceptions() { + assertRebroadcastToAllExceptSignerAndSuppliedExceptions( + TestHelpers::createSignedCommitPayload, CommitMessageData::create, ibftGossip::gossipCommitPayload); } @Test - public void rebroadcastsCommitOnlyOnce() { - rebroadcastOnlyOnce( - this::createSignedCommitPayload, + public void assertRebroadcastsCommitOnlyOnce() { + assertRebroadcastOnlyOnce( + TestHelpers::createSignedCommitPayload, CommitMessageData::create, ibftGossip::gossipCommitPayload); } @Test - public void rebroadcastsRoundChangeToAllExceptSigner() { - rebroadcastToAllExceptSigner( - this::createSignedRoundChangePayload, + public void assertRebroadcastsRoundChangeToAllExceptSigner() { + assertRebroadcastToAllExceptSigner( + TestHelpers::createSignedRoundChangePayload, RoundChangeMessageData::create, ibftGossip::gossipRoundChangePayload); } @Test - public void rebroadcastsRoundChangeToAllExceptSignerAndSuppliedExceptions() { - rebroadcastToAllExceptSignerAndSuppliedExceptions( - this::createSignedRoundChangePayload, + public void assertRebroadcastsRoundChangeToAllExceptSignerAndSuppliedExceptions() { + assertRebroadcastToAllExceptSignerAndSuppliedExceptions( + TestHelpers::createSignedRoundChangePayload, RoundChangeMessageData::create, ibftGossip::gossipRoundChangePayload); } @Test - public void rebroadcastsRoundChangeOnlyOnce() { - rebroadcastOnlyOnce( - this::createSignedRoundChangePayload, + public void assertRebroadcastsRoundChangeOnlyOnce() { + assertRebroadcastOnlyOnce( + TestHelpers::createSignedRoundChangePayload, RoundChangeMessageData::create, ibftGossip::gossipRoundChangePayload); } @Test - public void rebroadcastsNewRoundToAllExceptSigner() { - rebroadcastToAllExceptSigner( - this::createSignedNewRoundPayload, + public void assertRebroadcastsNewRoundToAllExceptSigner() { + assertRebroadcastToAllExceptSigner( + TestHelpers::createSignedNewRoundPayload, NewRoundMessageData::create, ibftGossip::gossipNewRoundPayload); } @Test - public void rebroadcastsNewRoundToAllExceptSignerAndSuppliedExceptions() { - rebroadcastToAllExceptSignerAndSuppliedExceptions( - this::createSignedNewRoundPayload, + public void assertRebroadcastsNewRoundToAllExceptSignerAndSuppliedExceptions() { + assertRebroadcastToAllExceptSignerAndSuppliedExceptions( + TestHelpers::createSignedNewRoundPayload, NewRoundMessageData::create, ibftGossip::gossipNewRoundPayload); } @Test - public void rebroadcastsNewRoundOnlyOnce() { - rebroadcastOnlyOnce( - this::createSignedNewRoundPayload, + public void assertRebroadcastsNewRoundOnlyOnce() { + assertRebroadcastOnlyOnce( + TestHelpers::createSignedNewRoundPayload, NewRoundMessageData::create, ibftGossip::gossipNewRoundPayload); } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/TestHelpers.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/TestHelpers.java index fb208e7357..1b6423e542 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/TestHelpers.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/TestHelpers.java @@ -12,10 +12,26 @@ */ package tech.pegasys.pantheon.consensus.ibft; +import static com.google.common.collect.Lists.newArrayList; +import static java.util.Collections.singletonList; + +import java.math.BigInteger; +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.PreparePayload; +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.KeyPair; +import tech.pegasys.pantheon.crypto.SECP256K1.Signature; import tech.pegasys.pantheon.ethereum.core.Address; +import tech.pegasys.pantheon.ethereum.core.AddressHelpers; import tech.pegasys.pantheon.ethereum.core.Block; import tech.pegasys.pantheon.ethereum.core.BlockDataGenerator; import tech.pegasys.pantheon.ethereum.core.BlockDataGenerator.BlockOptions; +import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.util.bytes.BytesValue; import java.util.Collections; @@ -45,4 +61,47 @@ public static Block createProposalBlock(final List

validators, final in .setBlockHashFunction(IbftBlockHashing::calculateDataHashForCommittedSeal); return new BlockDataGenerator().block(blockOptions); } + + public static SignedData createSignedProposalPayload(final KeyPair signerKeys) { + final MessageFactory messageFactory = new MessageFactory(signerKeys); + final ConsensusRoundIdentifier roundIdentifier = + new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); + final Block block = + TestHelpers.createProposalBlock(singletonList(AddressHelpers.ofValue(1)), 0); + return messageFactory.createSignedProposalPayload(roundIdentifier, block); + } + + public static SignedData createSignedPreparePayload(final KeyPair signerKeys) { + final MessageFactory messageFactory = new MessageFactory(signerKeys); + final ConsensusRoundIdentifier roundIdentifier = + new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); + return messageFactory.createSignedPreparePayload( + roundIdentifier, Hash.fromHexStringLenient("0")); + } + + public static SignedData createSignedCommitPayload(final KeyPair signerKeys) { + final MessageFactory messageFactory = new MessageFactory(signerKeys); + final ConsensusRoundIdentifier roundIdentifier = + new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); + return messageFactory.createSignedCommitPayload( + roundIdentifier, + Hash.fromHexStringLenient("0"), + Signature.create(BigInteger.ONE, BigInteger.TEN, (byte) 0)); + } + + public static SignedData createSignedRoundChangePayload(final KeyPair signerKeys) { + final MessageFactory messageFactory = new MessageFactory(signerKeys); + final ConsensusRoundIdentifier roundIdentifier = + new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); + return messageFactory.createSignedRoundChangePayload(roundIdentifier, Optional.empty()); + } + + public static SignedData createSignedNewRoundPayload(final KeyPair signerKeys) { + final MessageFactory messageFactory = new MessageFactory(signerKeys); + final ConsensusRoundIdentifier roundIdentifier = + new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); + final SignedData proposalPayload = createSignedProposalPayload(signerKeys); + return messageFactory.createSignedNewRoundPayload( + roundIdentifier, new RoundChangeCertificate(newArrayList()), proposalPayload); + } } From 64ff2ddd2f0cff1214afc231dcab7f9202992201 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Fri, 4 Jan 2019 14:29:15 +1000 Subject: [PATCH 22/33] [NC-1909] Added test for seen messages eviction --- .../pantheon/consensus/ibft/IbftGossip.java | 11 +++-- .../consensus/ibft/IbftGossipTest.java | 44 ++++++++++++++++++- .../pantheon/consensus/ibft/TestHelpers.java | 6 ++- 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java index 4a6790ba98..ad58a0070a 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java @@ -41,7 +41,7 @@ public class IbftGossip { private final IbftNetworkPeers peers; // Size of the seenMessages cache, should end up utilising 65bytes * this number + some meta data - private static final int SEEN_MESSAGES_SIZE = 10_000; + private final int seenMessagesSize; // Set that starts evicting members when it hits capacity private final Set seenMessages = @@ -49,17 +49,22 @@ public class IbftGossip { new LinkedHashMap() { @Override protected boolean removeEldestEntry(final Map.Entry eldest) { - return size() > SEEN_MESSAGES_SIZE; + return size() > seenMessagesSize; } }); + IbftGossip(final IbftNetworkPeers peers, final int seenMessagesSize) { + this.seenMessagesSize = seenMessagesSize; + this.peers = peers; + } + /** * Constructor that attaches gossip logic to a set of peers * * @param peers The always up to date set of connected peers that understand IBFT */ public IbftGossip(final IbftNetworkPeers peers) { - this.peers = peers; + this(peers, 10_000); } /** diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java index bd0f1bc373..655caf99b3 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java @@ -23,6 +23,7 @@ import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.Payload; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; import tech.pegasys.pantheon.consensus.ibft.network.IbftNetworkPeers; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; @@ -46,7 +47,7 @@ public class IbftGossipTest { @Before public void setup() { - ibftGossip = new IbftGossip(ibftNetworkPeers); + ibftGossip = new IbftGossip(ibftNetworkPeers, 10); } @@ -212,4 +213,45 @@ public void assertRebroadcastsNewRoundOnlyOnce() { NewRoundMessageData::create, ibftGossip::gossipNewRoundPayload); } + + @Test + public void evictMessageRecordAtCapacity() { + final KeyPair keypair = KeyPair.generate(); + final SignedData payload = TestHelpers.createSignedProposalPayloadWithRound(keypair, 0); + final boolean gossip1Result = ibftGossip.gossipProposalPayload(payload); + final boolean gossip2Result = ibftGossip.gossipProposalPayload(payload); + assertThat(gossip1Result).isTrue(); + assertThat(gossip2Result).isFalse(); + final MessageData messageData = ProposalMessageData.create(payload); + verify(ibftNetworkPeers, times(1)) + .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + + for (int i = 1; i <= 9; i++) { + final SignedData nextPayload = TestHelpers.createSignedProposalPayloadWithRound(keypair, i); + final boolean nextGossipResult = ibftGossip.gossipProposalPayload(nextPayload); + assertThat(nextGossipResult).isTrue(); + } + + final boolean gossip3Result = ibftGossip.gossipProposalPayload(payload); + assertThat(gossip3Result).isFalse(); + verify(ibftNetworkPeers, times(1)) + .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + + { + final SignedData nextPayload = + TestHelpers.createSignedProposalPayloadWithRound(keypair, 10); + final boolean nextGossipResult = ibftGossip.gossipProposalPayload(nextPayload); + assertThat(nextGossipResult).isTrue(); + } + + final boolean gossip4Result = ibftGossip.gossipProposalPayload(payload); + assertThat(gossip4Result).isTrue(); + verify(ibftNetworkPeers, times(2)) + .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + + final boolean gossip5Result = ibftGossip.gossipProposalPayload(payload); + assertThat(gossip5Result).isFalse(); + verify(ibftNetworkPeers, times(2)) + .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/TestHelpers.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/TestHelpers.java index 1b6423e542..b1a527d86e 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/TestHelpers.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/TestHelpers.java @@ -63,9 +63,13 @@ public static Block createProposalBlock(final List
validators, final in } public static SignedData createSignedProposalPayload(final KeyPair signerKeys) { + return createSignedProposalPayloadWithRound(signerKeys, 0xFEDCBA98); + } + + public static SignedData createSignedProposalPayloadWithRound(final KeyPair signerKeys, final int round) { final MessageFactory messageFactory = new MessageFactory(signerKeys); final ConsensusRoundIdentifier roundIdentifier = - new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); + new ConsensusRoundIdentifier(0x1234567890ABCDEFL, round); final Block block = TestHelpers.createProposalBlock(singletonList(AddressHelpers.ofValue(1)), 0); return messageFactory.createSignedProposalPayload(roundIdentifier, block); From 5b4b4c7ad9207e2229f677e6d5e29353523f9303 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Fri, 4 Jan 2019 14:37:16 +1000 Subject: [PATCH 23/33] [NC-1909] Spotless --- .../pegasys/pantheon/consensus/ibft/IbftGossipTest.java | 7 ++++--- .../tech/pegasys/pantheon/consensus/ibft/TestHelpers.java | 8 +++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java index 655caf99b3..7c90e6cfd7 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java @@ -50,7 +50,6 @@ public void setup() { ibftGossip = new IbftGossip(ibftNetworkPeers, 10); } - private

void assertRebroadcastToAllExceptSigner( final Function> createPayload, final Function, MessageData> createMessageData, @@ -217,7 +216,8 @@ public void assertRebroadcastsNewRoundOnlyOnce() { @Test public void evictMessageRecordAtCapacity() { final KeyPair keypair = KeyPair.generate(); - final SignedData payload = TestHelpers.createSignedProposalPayloadWithRound(keypair, 0); + final SignedData payload = + TestHelpers.createSignedProposalPayloadWithRound(keypair, 0); final boolean gossip1Result = ibftGossip.gossipProposalPayload(payload); final boolean gossip2Result = ibftGossip.gossipProposalPayload(payload); assertThat(gossip1Result).isTrue(); @@ -227,7 +227,8 @@ public void evictMessageRecordAtCapacity() { .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); for (int i = 1; i <= 9; i++) { - final SignedData nextPayload = TestHelpers.createSignedProposalPayloadWithRound(keypair, i); + final SignedData nextPayload = + TestHelpers.createSignedProposalPayloadWithRound(keypair, i); final boolean nextGossipResult = ibftGossip.gossipProposalPayload(nextPayload); assertThat(nextGossipResult).isTrue(); } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/TestHelpers.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/TestHelpers.java index b1a527d86e..2104acec1d 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/TestHelpers.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/TestHelpers.java @@ -15,7 +15,6 @@ import static com.google.common.collect.Lists.newArrayList; import static java.util.Collections.singletonList; -import java.math.BigInteger; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.CommitPayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.MessageFactory; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.NewRoundPayload; @@ -34,6 +33,7 @@ import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.util.bytes.BytesValue; +import java.math.BigInteger; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -66,7 +66,8 @@ public static SignedData createSignedProposalPayload(final KeyP return createSignedProposalPayloadWithRound(signerKeys, 0xFEDCBA98); } - public static SignedData createSignedProposalPayloadWithRound(final KeyPair signerKeys, final int round) { + public static SignedData createSignedProposalPayloadWithRound( + final KeyPair signerKeys, final int round) { final MessageFactory messageFactory = new MessageFactory(signerKeys); final ConsensusRoundIdentifier roundIdentifier = new ConsensusRoundIdentifier(0x1234567890ABCDEFL, round); @@ -93,7 +94,8 @@ public static SignedData createSignedCommitPayload(final KeyPair Signature.create(BigInteger.ONE, BigInteger.TEN, (byte) 0)); } - public static SignedData createSignedRoundChangePayload(final KeyPair signerKeys) { + public static SignedData createSignedRoundChangePayload( + final KeyPair signerKeys) { final MessageFactory messageFactory = new MessageFactory(signerKeys); final ConsensusRoundIdentifier roundIdentifier = new ConsensusRoundIdentifier(0x1234567890ABCDEFL, 0xFEDCBA98); From 9ff8f81e3e9618cea6ce1c00179a93930c8001d5 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Mon, 7 Jan 2019 08:39:11 +1000 Subject: [PATCH 24/33] [NC-1909] deleted unused function after refactor --- .../consensus/ibft/ibftevent/IbftReceivedMessageEvent.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftReceivedMessageEvent.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftReceivedMessageEvent.java index aac285b273..11960b1463 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftReceivedMessageEvent.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftReceivedMessageEvent.java @@ -24,10 +24,6 @@ public IbftReceivedMessageEvent(final Message message) { this.message = message; } - public MessageData getMessageData() { - return message.getData(); - } - public Message getMessage() { return message; } From 008560651af5bb8fb998c27d11eeb37aab9fb5e9 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Mon, 7 Jan 2019 08:40:23 +1000 Subject: [PATCH 25/33] [NC-1909] variable that should have been final --- .../pantheon/consensus/ibft/statemachine/IbftController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git 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 index df440ba1b7..f78868cb13 100644 --- 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 @@ -173,7 +173,7 @@ private void startNewHeightManager(final BlockHeader parentHeader) { currentHeightManager = ibftBlockHeightManagerFactory.create(parentHeader); currentHeightManager.start(); final long newChainHeight = currentHeightManager.getChainHeight(); - List orDefault = futureMessages.getOrDefault(newChainHeight, emptyList()); + final List orDefault = futureMessages.getOrDefault(newChainHeight, emptyList()); orDefault.forEach(this::handleMessage); futureMessages.remove(newChainHeight); } From 810aeb5d17dcb402b30aa6ffbc149ceff97d0ab6 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Mon, 7 Jan 2019 09:01:04 +1000 Subject: [PATCH 26/33] [NC-1909] extracted controller case logic into common function --- .../ibft/statemachine/IbftController.java | 82 +++++++++++-------- 1 file changed, 47 insertions(+), 35 deletions(-) diff --git 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 index f78868cb13..0ee28791ef 100644 --- 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 @@ -14,12 +14,16 @@ import static java.util.Collections.emptyList; +import java.util.function.BiFunction; +import java.util.function.Consumer; +import java.util.function.Function; import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; import tech.pegasys.pantheon.consensus.ibft.IbftGossip; import tech.pegasys.pantheon.consensus.ibft.ibftevent.BlockTimerExpiry; import tech.pegasys.pantheon.consensus.ibft.ibftevent.IbftReceivedMessageEvent; import tech.pegasys.pantheon.consensus.ibft.ibftevent.NewChainHead; import tech.pegasys.pantheon.consensus.ibft.ibftevent.RoundExpiry; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.AbstractIbftMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.IbftV2; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessageData; @@ -34,6 +38,7 @@ import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangePayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; import tech.pegasys.pantheon.ethereum.chain.Blockchain; +import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.p2p.api.Message; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; @@ -94,53 +99,48 @@ private void handleMessage(final Message message) { final MessageData messageData = message.getData(); switch (messageData.getCode()) { case IbftV2.PROPOSAL: - final SignedData signedProposalPayload = - ProposalMessageData.fromMessageData(messageData).decode(); - if (processMessage(signedProposalPayload, message)) { - gossiper.gossipProposalPayload( - signedProposalPayload, message.getConnection().getPeer().getAddress()); - currentHeightManager.handleProposalMessage(signedProposalPayload); - } + processMessage( + message, + ProposalMessageData.fromMessageData(messageData).decode(), + gossiper::gossipProposalPayload, + currentHeightManager::handleProposalMessage + ); break; case IbftV2.PREPARE: - final SignedData signedPreparePayload = - PrepareMessageData.fromMessageData(messageData).decode(); - if (processMessage(signedPreparePayload, message)) { - gossiper.gossipPreparePayload( - signedPreparePayload, message.getConnection().getPeer().getAddress()); - currentHeightManager.handlePrepareMessage(signedPreparePayload); - } + processMessage( + message, + PrepareMessageData.fromMessageData(messageData).decode(), + gossiper::gossipPreparePayload, + currentHeightManager::handlePrepareMessage + ); break; case IbftV2.COMMIT: - final SignedData signedCommitPayload = - CommitMessageData.fromMessageData(messageData).decode(); - if (processMessage(signedCommitPayload, message)) { - gossiper.gossipCommitPayload( - signedCommitPayload, message.getConnection().getPeer().getAddress()); - currentHeightManager.handleCommitMessage(signedCommitPayload); - } + processMessage( + message, + CommitMessageData.fromMessageData(messageData).decode(), + gossiper::gossipCommitPayload, + currentHeightManager::handleCommitMessage + ); break; case IbftV2.ROUND_CHANGE: - final SignedData signedRoundChangePayload = - RoundChangeMessageData.fromMessageData(messageData).decode(); - if (processMessage(signedRoundChangePayload, message)) { - gossiper.gossipRoundChangePayload( - signedRoundChangePayload, message.getConnection().getPeer().getAddress()); - currentHeightManager.handleRoundChangeMessage(signedRoundChangePayload); - } + processMessage( + message, + RoundChangeMessageData.fromMessageData(messageData).decode(), + gossiper::gossipRoundChangePayload, + currentHeightManager::handleRoundChangeMessage + ); break; case IbftV2.NEW_ROUND: - final SignedData signedNewRoundPayload = - NewRoundMessageData.fromMessageData(messageData).decode(); - if (processMessage(signedNewRoundPayload, message)) { - gossiper.gossipNewRoundPayload( - signedNewRoundPayload, message.getConnection().getPeer().getAddress()); - currentHeightManager.handleNewRoundMessage(signedNewRoundPayload); - } + processMessage( + message, + NewRoundMessageData.fromMessageData(messageData).decode(), + gossiper::gossipNewRoundPayload, + currentHeightManager::handleNewRoundMessage + ); break; default: @@ -149,6 +149,18 @@ private void handleMessage(final Message message) { } } + private

void processMessage( + final Message message, + final SignedData

signedPayload, + final BiFunction, Address, Boolean> gossipMessage, + final Consumer> handleMessage + ) { + if (processMessage(signedPayload, message)) { + gossipMessage.apply(signedPayload, message.getConnection().getPeer().getAddress()); + handleMessage.accept(signedPayload); + } + } + public void handleNewBlockEvent(final NewChainHead newChainHead) { startNewHeightManager(newChainHead.getNewChainHeadHeader()); } From be718f1e7331426be8fb28c4c18a74909c128c0c Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Mon, 7 Jan 2019 09:05:15 +1000 Subject: [PATCH 27/33] [NC-1909] Fixed collection not being copied before filtering --- .../ibftevent/IbftReceivedMessageEvent.java | 1 - .../ibft/network/IbftNetworkPeers.java | 11 +++++-- .../ibft/statemachine/IbftController.java | 29 +++++-------------- 3 files changed, 16 insertions(+), 25 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftReceivedMessageEvent.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftReceivedMessageEvent.java index 11960b1463..254a7879cb 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftReceivedMessageEvent.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftevent/IbftReceivedMessageEvent.java @@ -14,7 +14,6 @@ import tech.pegasys.pantheon.consensus.ibft.ibftevent.IbftEvents.Type; import tech.pegasys.pantheon.ethereum.p2p.api.Message; -import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; public class IbftReceivedMessageEvent implements IbftEvent { diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java index 2e2c9f91c1..a91ef99504 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java @@ -21,6 +21,7 @@ import java.util.Collection; import java.util.Map; import java.util.Objects; +import java.util.stream.Collectors; import com.google.common.collect.Maps; import org.apache.logging.log4j.LogManager; @@ -56,9 +57,13 @@ public void multicastToValidators(final MessageData message) { public void multicastToValidatorsExcept( final MessageData message, final Collection

exceptAddresses) { - final Collection
validators = validatorProvider.getValidators(); - validators.removeIf(exceptAddresses::contains); - sendMessageToSpecificAddresses(validators, message); + final Collection
includedValidators = + validatorProvider + .getValidators() + .stream() + .filter(exceptAddresses::contains) + .collect(Collectors.toSet()); + sendMessageToSpecificAddresses(includedValidators, message); } private void sendMessageToSpecificAddresses( diff --git 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 index 0ee28791ef..87b808155d 100644 --- 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 @@ -14,28 +14,19 @@ import static java.util.Collections.emptyList; -import java.util.function.BiFunction; -import java.util.function.Consumer; -import java.util.function.Function; import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; import tech.pegasys.pantheon.consensus.ibft.IbftGossip; import tech.pegasys.pantheon.consensus.ibft.ibftevent.BlockTimerExpiry; import tech.pegasys.pantheon.consensus.ibft.ibftevent.IbftReceivedMessageEvent; import tech.pegasys.pantheon.consensus.ibft.ibftevent.NewChainHead; import tech.pegasys.pantheon.consensus.ibft.ibftevent.RoundExpiry; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.AbstractIbftMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.IbftV2; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessageData; -import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.CommitPayload; -import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.NewRoundPayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.Payload; -import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparePayload; -import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload; -import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangePayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; import tech.pegasys.pantheon.ethereum.chain.Blockchain; import tech.pegasys.pantheon.ethereum.core.Address; @@ -45,6 +36,8 @@ import java.util.List; import java.util.Map; +import java.util.function.BiFunction; +import java.util.function.Consumer; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; @@ -103,8 +96,7 @@ private void handleMessage(final Message message) { message, ProposalMessageData.fromMessageData(messageData).decode(), gossiper::gossipProposalPayload, - currentHeightManager::handleProposalMessage - ); + currentHeightManager::handleProposalMessage); break; case IbftV2.PREPARE: @@ -112,8 +104,7 @@ private void handleMessage(final Message message) { message, PrepareMessageData.fromMessageData(messageData).decode(), gossiper::gossipPreparePayload, - currentHeightManager::handlePrepareMessage - ); + currentHeightManager::handlePrepareMessage); break; case IbftV2.COMMIT: @@ -121,8 +112,7 @@ private void handleMessage(final Message message) { message, CommitMessageData.fromMessageData(messageData).decode(), gossiper::gossipCommitPayload, - currentHeightManager::handleCommitMessage - ); + currentHeightManager::handleCommitMessage); break; case IbftV2.ROUND_CHANGE: @@ -130,8 +120,7 @@ private void handleMessage(final Message message) { message, RoundChangeMessageData.fromMessageData(messageData).decode(), gossiper::gossipRoundChangePayload, - currentHeightManager::handleRoundChangeMessage - ); + currentHeightManager::handleRoundChangeMessage); break; case IbftV2.NEW_ROUND: @@ -139,8 +128,7 @@ private void handleMessage(final Message message) { message, NewRoundMessageData.fromMessageData(messageData).decode(), gossiper::gossipNewRoundPayload, - currentHeightManager::handleNewRoundMessage - ); + currentHeightManager::handleNewRoundMessage); break; default: @@ -153,8 +141,7 @@ private

void processMessage( final Message message, final SignedData

signedPayload, final BiFunction, Address, Boolean> gossipMessage, - final Consumer> handleMessage - ) { + final Consumer> handleMessage) { if (processMessage(signedPayload, message)) { gossipMessage.apply(signedPayload, message.getConnection().getPeer().getAddress()); handleMessage.accept(signedPayload); From 591833d97265d450ca4a1c30f8f98da0da7ef7e8 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Mon, 7 Jan 2019 09:06:44 +1000 Subject: [PATCH 28/33] [NC-1909] renamed variable at suggestion --- .../tech/pegasys/pantheon/consensus/ibft/IbftGossip.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java index ad58a0070a..1adefe349d 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java @@ -41,7 +41,7 @@ public class IbftGossip { private final IbftNetworkPeers peers; // Size of the seenMessages cache, should end up utilising 65bytes * this number + some meta data - private final int seenMessagesSize; + private final int maxSeenMessages; // Set that starts evicting members when it hits capacity private final Set seenMessages = @@ -49,12 +49,12 @@ public class IbftGossip { new LinkedHashMap() { @Override protected boolean removeEldestEntry(final Map.Entry eldest) { - return size() > seenMessagesSize; + return size() > maxSeenMessages; } }); - IbftGossip(final IbftNetworkPeers peers, final int seenMessagesSize) { - this.seenMessagesSize = seenMessagesSize; + IbftGossip(final IbftNetworkPeers peers, final int maxSeenMessages) { + this.maxSeenMessages = maxSeenMessages; this.peers = peers; } From 64ae2652bffa3a00e287e8d53895c0dace74ff54 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Mon, 7 Jan 2019 09:13:53 +1000 Subject: [PATCH 29/33] [NC-1909] method renames to reflect them operating on payloads not messages --- .../statemachine/IbftBlockHeightManager.java | 46 +++++++++---------- .../ibft/statemachine/IbftController.java | 10 ++-- .../IbftBlockHeightManagerTest.java | 14 +++--- .../ibft/statemachine/IbftControllerTest.java | 30 ++++++------ 4 files changed, 50 insertions(+), 50 deletions(-) diff --git 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 index 1c29332cd0..aa9193c7a3 100644 --- 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 @@ -146,24 +146,24 @@ public void roundExpired(final RoundExpiry expire) { // Its possible the locally created RoundChange triggers the transmission of a NewRound // message - so it must be handled accordingly. - handleRoundChangeMessage(localRoundChange); + handleRoundChangePayload(localRoundChange); } - public void handleProposalMessage(final SignedData msgData) { - LOG.info("Received a Proposal message."); + public void handleProposalPayload(final SignedData signedPayload) { + LOG.info("Received a Proposal Payload."); actionOrBufferMessage( - msgData, currentRound::handleProposalMessage, RoundState::setProposedBlock); + signedPayload, currentRound::handleProposalMessage, RoundState::setProposedBlock); } - public void handlePrepareMessage(final SignedData msgData) { - LOG.info("Received a prepare message."); + public void handlePreparePayload(final SignedData signedPayload) { + LOG.info("Received a prepare Payload."); actionOrBufferMessage( - msgData, currentRound::handlePrepareMessage, RoundState::addPrepareMessage); + signedPayload, currentRound::handlePrepareMessage, RoundState::addPrepareMessage); } - public void handleCommitMessage(final SignedData msgData) { - LOG.info("Received a commit message."); - actionOrBufferMessage(msgData, currentRound::handleCommitMessage, RoundState::addCommitMessage); + public void handleCommitPayload(final SignedData payload) { + LOG.info("Received a commit Payload."); + actionOrBufferMessage(payload, currentRound::handleCommitMessage, RoundState::addCommitMessage); } private void actionOrBufferMessage( @@ -183,16 +183,16 @@ private void actionOrBufferMessage( } } - public void handleRoundChangeMessage(final SignedData msgData) { + public void handleRoundChangePayload(final SignedData signedPayload) { final Optional result = - roundChangeManager.appendRoundChangeMessage(msgData); - final MessageAge messageAge = determineAgeOfPayload(msgData.getPayload()); + roundChangeManager.appendRoundChangeMessage(signedPayload); + final MessageAge messageAge = determineAgeOfPayload(signedPayload.getPayload()); if (messageAge == PRIOR_ROUND) { - LOG.info("Received RoundChange Message for a prior round."); + LOG.info("Received RoundChange Payload for a prior round."); return; } - ConsensusRoundIdentifier targetRound = msgData.getPayload().getRoundIdentifier(); - LOG.info("Received a RoundChange message for {}", targetRound.toString()); + final ConsensusRoundIdentifier targetRound = signedPayload.getPayload().getRoundIdentifier(); + LOG.info("Received a RoundChange Payload for {}", targetRound.toString()); if (result.isPresent()) { if (messageAge == FUTURE_ROUND) { @@ -220,17 +220,17 @@ private void startNewRound(final int roundNumber) { roundTimer.startTimer(currentRound.getRoundIdentifier()); } - public void handleNewRoundMessage(final SignedData msgData) { - final NewRoundPayload payload = msgData.getPayload(); + public void handleNewRoundPayload(final SignedData signedPayload) { + final NewRoundPayload payload = signedPayload.getPayload(); final MessageAge messageAge = determineAgeOfPayload(payload); if (messageAge == PRIOR_ROUND) { - LOG.info("Received NewRound Message for a prior round."); + LOG.info("Received NewRound Payload for a prior round."); return; } - LOG.info("Received NewRound Message for {}", payload.getRoundIdentifier().toString()); + LOG.info("Received NewRound Payload for {}", payload.getRoundIdentifier().toString()); - if (newRoundMessageValidator.validateNewRoundMessage(msgData)) { + if (newRoundMessageValidator.validateNewRoundMessage(signedPayload)) { if (messageAge == FUTURE_ROUND) { startNewRound(payload.getRoundIdentifier().getRoundNumber()); } @@ -243,8 +243,8 @@ public long getChainHeight() { } private MessageAge determineAgeOfPayload(final Payload payload) { - int messageRoundNumber = payload.getRoundIdentifier().getRoundNumber(); - int currentRoundNumber = currentRound.getRoundIdentifier().getRoundNumber(); + final int messageRoundNumber = payload.getRoundIdentifier().getRoundNumber(); + final int currentRoundNumber = currentRound.getRoundIdentifier().getRoundNumber(); if (messageRoundNumber > currentRoundNumber) { return FUTURE_ROUND; } else if (messageRoundNumber == currentRoundNumber) { diff --git 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 index 87b808155d..19c94503cc 100644 --- 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 @@ -96,7 +96,7 @@ private void handleMessage(final Message message) { message, ProposalMessageData.fromMessageData(messageData).decode(), gossiper::gossipProposalPayload, - currentHeightManager::handleProposalMessage); + currentHeightManager::handleProposalPayload); break; case IbftV2.PREPARE: @@ -104,7 +104,7 @@ private void handleMessage(final Message message) { message, PrepareMessageData.fromMessageData(messageData).decode(), gossiper::gossipPreparePayload, - currentHeightManager::handlePrepareMessage); + currentHeightManager::handlePreparePayload); break; case IbftV2.COMMIT: @@ -112,7 +112,7 @@ private void handleMessage(final Message message) { message, CommitMessageData.fromMessageData(messageData).decode(), gossiper::gossipCommitPayload, - currentHeightManager::handleCommitMessage); + currentHeightManager::handleCommitPayload); break; case IbftV2.ROUND_CHANGE: @@ -120,7 +120,7 @@ private void handleMessage(final Message message) { message, RoundChangeMessageData.fromMessageData(messageData).decode(), gossiper::gossipRoundChangePayload, - currentHeightManager::handleRoundChangeMessage); + currentHeightManager::handleRoundChangePayload); break; case IbftV2.NEW_ROUND: @@ -128,7 +128,7 @@ private void handleMessage(final Message message) { message, NewRoundMessageData.fromMessageData(messageData).decode(), gossiper::gossipNewRoundPayload, - currentHeightManager::handleNewRoundMessage); + currentHeightManager::handleNewRoundPayload); break; default: diff --git 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 index 2475399490..713b2a38d9 100644 --- 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 @@ -233,7 +233,7 @@ public void onRoundChangeReceptionRoundChangeManagerIsInvokedAndNewRoundStarted( manager.start(); verify(roundFactory).createNewRound(any(), eq(0)); - manager.handleRoundChangeMessage(roundChangePayload); + manager.handleRoundChangePayload(roundChangePayload); verify(roundChangeManager, times(1)).appendRoundChangeMessage(roundChangePayload); verify(roundFactory, times(1)) @@ -279,7 +279,7 @@ public void whenSufficientRoundChangesAreReceivedANewRoundMessageIsTransmitted() messageValidatorFactory); manager.start(); - manager.handleRoundChangeMessage(roundChangePayload); + manager.handleRoundChangePayload(roundChangePayload); verify(messageTransmitter, times(1)) .multicastNewRound(eq(futureRoundIdentifier), eq(roundChangCert), any()); @@ -311,8 +311,8 @@ public void messagesForFutureRoundsAreBufferedAndUsedToPreloadNewRoundWhenItIsSt Hash.fromHexStringLenient("0"), Signature.create(BigInteger.ONE, BigInteger.ONE, (byte) 1)); - manager.handlePrepareMessage(preparePayload); - manager.handleCommitMessage(commitPayload); + manager.handlePreparePayload(preparePayload); + manager.handleCommitPayload(commitPayload); // Force a new round to be started at new round number. final SignedData newRound = @@ -321,7 +321,7 @@ public void messagesForFutureRoundsAreBufferedAndUsedToPreloadNewRoundWhenItIsSt new RoundChangeCertificate(Collections.emptyList()), messageFactory.createSignedProposalPayload(futureRoundIdentifier, createdBlock)); - manager.handleNewRoundMessage(newRound); + manager.handleNewRoundPayload(newRound); // Final state sets the Quorum Size to 3, so should send a Prepare and also a commit verify(messageTransmitter, times(1)).multicastPrepare(eq(futureRoundIdentifier), any()); @@ -349,8 +349,8 @@ public void preparedCertificateIncludedInRoundChangeMessageOnRoundTimeoutExpired validatorMessageFactory .get(1) .createSignedPreparePayload(roundIdentifier, Hash.fromHexStringLenient("0")); - manager.handlePrepareMessage(preparePayload); - manager.handlePrepareMessage(secondPreparePayload); + manager.handlePreparePayload(preparePayload); + manager.handlePreparePayload(secondPreparePayload); manager.roundExpired(new RoundExpiry(roundIdentifier)); diff --git 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 index 760b03d315..79b6827b98 100644 --- 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 @@ -138,14 +138,14 @@ public void startsNewBlockHeightManagerAndReplaysFutureMessages() { verify(blockHeightManagerFactory).create(blockHeader); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); - verify(blockHeightManager, never()).handleProposalMessage(signedProposal); - verify(blockHeightManager).handlePrepareMessage(signedPrepare); + verify(blockHeightManager, never()).handleProposalPayload(signedProposal); + verify(blockHeightManager).handlePreparePayload(signedPrepare); verify(ibftGossip).gossipPreparePayload(signedPrepare, (Address) null); - verify(blockHeightManager).handleCommitMessage(signedCommit); + verify(blockHeightManager).handleCommitPayload(signedCommit); verify(ibftGossip).gossipCommitPayload(signedCommit, (Address) null); - verify(blockHeightManager).handleRoundChangeMessage(signedRoundChange); + verify(blockHeightManager).handleRoundChangePayload(signedRoundChange); verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, (Address) null); - verify(blockHeightManager, never()).handleNewRoundMessage(signedNewRound); + verify(blockHeightManager, never()).handleNewRoundPayload(signedNewRound); } @Test @@ -168,15 +168,15 @@ public void createsNewBlockHeightManagerAndReplaysFutureMessagesOnNewChainHeadEv verify(blockHeightManagerFactory).create(blockHeader); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); - verify(blockHeightManager).handleProposalMessage(signedProposal); + verify(blockHeightManager).handleProposalPayload(signedProposal); verify(ibftGossip).gossipProposalPayload(signedProposal, (Address) null); - verify(blockHeightManager).handlePrepareMessage(signedPrepare); + verify(blockHeightManager).handlePreparePayload(signedPrepare); verify(ibftGossip).gossipPreparePayload(signedPrepare, (Address) null); - verify(blockHeightManager).handleCommitMessage(signedCommit); + verify(blockHeightManager).handleCommitPayload(signedCommit); verify(ibftGossip).gossipCommitPayload(signedCommit, (Address) null); - verify(blockHeightManager).handleRoundChangeMessage(signedRoundChange); + verify(blockHeightManager).handleRoundChangePayload(signedRoundChange); verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, (Address) null); - verify(blockHeightManager).handleNewRoundMessage(signedNewRound); + verify(blockHeightManager).handleNewRoundPayload(signedNewRound); verify(ibftGossip).gossipNewRoundPayload(signedNewRound, (Address) null); } @@ -207,7 +207,7 @@ public void proposalForCurrentHeightIsPassedToBlockHeightManager() { ibftController.handleMessageEvent(new IbftReceivedMessageEvent(proposalMessage)); assertThat(futureMessages).isEmpty(); - verify(blockHeightManager).handleProposalMessage(signedProposal); + verify(blockHeightManager).handleProposalPayload(signedProposal); verify(ibftGossip).gossipProposalPayload(signedProposal, (Address) null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); @@ -221,7 +221,7 @@ public void prepareForCurrentHeightIsPassedToBlockHeightManager() { ibftController.handleMessageEvent(new IbftReceivedMessageEvent(prepareMessage)); assertThat(futureMessages).isEmpty(); - verify(blockHeightManager).handlePrepareMessage(signedPrepare); + verify(blockHeightManager).handlePreparePayload(signedPrepare); verify(ibftGossip).gossipPreparePayload(signedPrepare, (Address) null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); @@ -235,7 +235,7 @@ public void commitForCurrentHeightIsPassedToBlockHeightManager() { ibftController.handleMessageEvent(new IbftReceivedMessageEvent(commitMessage)); assertThat(futureMessages).isEmpty(); - verify(blockHeightManager).handleCommitMessage(signedCommit); + verify(blockHeightManager).handleCommitPayload(signedCommit); verify(ibftGossip).gossipCommitPayload(signedCommit, (Address) null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); @@ -250,7 +250,7 @@ public void newRoundForCurrentHeightIsPassedToBlockHeightManager() { ibftController.handleMessageEvent(new IbftReceivedMessageEvent(newRoundMessage)); assertThat(futureMessages).isEmpty(); - verify(blockHeightManager).handleNewRoundMessage(signedNewRound); + verify(blockHeightManager).handleNewRoundPayload(signedNewRound); verify(ibftGossip).gossipNewRoundPayload(signedNewRound, (Address) null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); @@ -265,7 +265,7 @@ public void roundChangeForCurrentHeightIsPassedToBlockHeightManager() { ibftController.handleMessageEvent(new IbftReceivedMessageEvent(roundChangeMessage)); assertThat(futureMessages).isEmpty(); - verify(blockHeightManager).handleRoundChangeMessage(signedRoundChange); + verify(blockHeightManager).handleRoundChangePayload(signedRoundChange); verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, (Address) null); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); From d9adf18b7352ab8266cc95f3ab8ce8298dc24b83 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Mon, 7 Jan 2019 09:18:30 +1000 Subject: [PATCH 30/33] [NC-1909] fixed errorprone warning about return value being unchecked --- .../consensus/ibft/statemachine/IbftController.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git 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 index 19c94503cc..12a63ad7a4 100644 --- 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 @@ -36,7 +36,7 @@ import java.util.List; import java.util.Map; -import java.util.function.BiFunction; +import java.util.function.BiConsumer; import java.util.function.Consumer; import com.google.common.annotations.VisibleForTesting; @@ -140,10 +140,10 @@ private void handleMessage(final Message message) { private

void processMessage( final Message message, final SignedData

signedPayload, - final BiFunction, Address, Boolean> gossipMessage, + final BiConsumer, Address> gossipMessage, final Consumer> handleMessage) { if (processMessage(signedPayload, message)) { - gossipMessage.apply(signedPayload, message.getConnection().getPeer().getAddress()); + gossipMessage.accept(signedPayload, message.getConnection().getPeer().getAddress()); handleMessage.accept(signedPayload); } } From 396258393f035e5e297e43cbb4e14abaa593d2da Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Mon, 7 Jan 2019 09:43:07 +1000 Subject: [PATCH 31/33] [NC-1909] Fixed inverted filter --- .../pantheon/consensus/ibft/network/IbftNetworkPeers.java | 2 +- .../pantheon/consensus/ibft/network/IbftNetworkPeersTest.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java index a91ef99504..55e4219bc0 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java @@ -61,7 +61,7 @@ public void multicastToValidatorsExcept( validatorProvider .getValidators() .stream() - .filter(exceptAddresses::contains) + .filter(a -> !exceptAddresses.contains(a)) .collect(Collectors.toSet()); sendMessageToSpecificAddresses(includedValidators, message); } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java index 7c40d7b999..f077ef790a 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeersTest.java @@ -110,6 +110,7 @@ public void onlyValidatorsAreSentAMessageNotInExcludes() throws PeerNotConnected // Only add the first Peer's address to the validators. final Address validatorAddress = Util.publicKeyToAddress(publicKeys.get(0)); validators.add(validatorAddress); + validators.add(Util.publicKeyToAddress(publicKeys.get(1))); final ValidatorProvider validatorProvider = mock(ValidatorProvider.class); when(validatorProvider.getValidators()).thenReturn(validators); @@ -122,7 +123,7 @@ public void onlyValidatorsAreSentAMessageNotInExcludes() throws PeerNotConnected peers.multicastToValidatorsExcept(messageToSend, newArrayList(validatorAddress)); verify(peerConnections.get(0), never()).sendForProtocol(any(), any()); - verify(peerConnections.get(1), never()).sendForProtocol(any(), any()); + verify(peerConnections.get(1), times(1)).sendForProtocol(any(), any()); verify(peerConnections.get(2), never()).sendForProtocol(any(), any()); verify(peerConnections.get(3), never()).sendForProtocol(any(), any()); } From 12faec48ed0cad66714a9fac70e1dd87618643dc Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Wed, 9 Jan 2019 12:18:08 +1000 Subject: [PATCH 32/33] [NC-1909] supporting integration tests --- .../ibft/support/MessageReceptionHelpers.java | 20 +++---- .../ibft/support/StubIbftMulticaster.java | 12 +++- .../ibft/support/StubbedPeerConnection.java | 58 +++++++++++++++++++ .../ibft/support/TestContextFactory.java | 10 +++- .../consensus/ibft/support/ValidatorPeer.java | 29 ++++++---- .../pantheon/consensus/ibft/IbftGossip.java | 8 +-- .../ibft/network/IbftMulticaster.java | 6 ++ .../ibft/network/IbftNetworkPeers.java | 1 + .../ibft/statemachine/IbftController.java | 2 +- .../controller/IbftPantheonController.java | 6 +- 10 files changed, 123 insertions(+), 29 deletions(-) create mode 100644 consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/StubbedPeerConnection.java diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/MessageReceptionHelpers.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/MessageReceptionHelpers.java index 0a922fce95..f76f454cff 100644 --- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/MessageReceptionHelpers.java +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/MessageReceptionHelpers.java @@ -14,12 +14,12 @@ import static org.assertj.core.api.Assertions.assertThat; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessage; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.IbftV2; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessage; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.Payload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; @@ -60,15 +60,15 @@ public static boolean msgMatchesExpected( switch (expectedPayload.getMessageType()) { case IbftV2.PROPOSAL: - return ProposalMessage.fromMessage(actual).decode().equals(expected); + return ProposalMessageData.fromMessageData(actual).decode().equals(expected); case IbftV2.PREPARE: - return PrepareMessage.fromMessage(actual).decode().equals(expected); + return PrepareMessageData.fromMessageData(actual).decode().equals(expected); case IbftV2.COMMIT: - return CommitMessage.fromMessage(actual).decode().equals(expected); + return CommitMessageData.fromMessageData(actual).decode().equals(expected); case IbftV2.NEW_ROUND: - return NewRoundMessage.fromMessage(actual).decode().equals(expected); + return NewRoundMessageData.fromMessageData(actual).decode().equals(expected); case IbftV2.ROUND_CHANGE: - return RoundChangeMessage.fromMessage(actual).decode().equals(expected); + return RoundChangeMessageData.fromMessageData(actual).decode().equals(expected); default: return false; } diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/StubIbftMulticaster.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/StubIbftMulticaster.java index 24f41065a2..57d123abd0 100644 --- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/StubIbftMulticaster.java +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/StubIbftMulticaster.java @@ -13,6 +13,7 @@ package tech.pegasys.pantheon.consensus.ibft.support; import tech.pegasys.pantheon.consensus.ibft.network.IbftMulticaster; +import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; import java.util.Collection; @@ -32,6 +33,15 @@ public void addNetworkPeers(final Collection nodes) { @Override public void multicastToValidators(final MessageData message) { - validatorNodes.forEach(v -> v.handleReceivedMessage(message)); + validatorNodes.forEach(peer -> peer.handleReceivedMessage(message)); + } + + @Override + public void multicastToValidatorsExcept( + final MessageData message, final Collection

exceptAddresses) { + validatorNodes + .stream() + .filter(peer -> !exceptAddresses.contains(peer.getNodeAddress())) + .forEach(peer -> peer.handleReceivedMessage(message)); } } diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/StubbedPeerConnection.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/StubbedPeerConnection.java new file mode 100644 index 0000000000..247a1566a6 --- /dev/null +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/StubbedPeerConnection.java @@ -0,0 +1,58 @@ +/* + * 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.support; + +import static java.util.Collections.emptyList; + +import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; +import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; +import tech.pegasys.pantheon.ethereum.p2p.wire.Capability; +import tech.pegasys.pantheon.ethereum.p2p.wire.PeerInfo; +import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage.DisconnectReason; +import tech.pegasys.pantheon.util.bytes.BytesValue; + +import java.net.SocketAddress; +import java.util.Set; + +public class StubbedPeerConnection implements PeerConnection { + + @Override + public void send(final Capability capability, final MessageData message) + throws PeerNotConnected {} + + @Override + public Set getAgreedCapabilities() { + return null; + } + + @Override + public PeerInfo getPeer() { + return new PeerInfo(0, "IbftIntTestPeer", emptyList(), 0, BytesValue.EMPTY); + } + + @Override + public void terminateConnection(final DisconnectReason reason, final boolean peerInitiated) {} + + @Override + public void disconnect(final DisconnectReason reason) {} + + @Override + public SocketAddress getLocalAddress() { + return null; + } + + @Override + public SocketAddress getRemoteAddress() { + return null; + } +} diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextFactory.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextFactory.java index 901660d2ee..bb5414f0a6 100644 --- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextFactory.java +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextFactory.java @@ -13,6 +13,7 @@ package tech.pegasys.pantheon.consensus.ibft.support; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.mockito.Mockito.mock; import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.createInMemoryBlockchain; import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.createInMemoryWorldStateArchive; @@ -29,6 +30,7 @@ import tech.pegasys.pantheon.consensus.ibft.IbftContext; import tech.pegasys.pantheon.consensus.ibft.IbftEventQueue; import tech.pegasys.pantheon.consensus.ibft.IbftExtraData; +import tech.pegasys.pantheon.consensus.ibft.IbftGossip; import tech.pegasys.pantheon.consensus.ibft.IbftHelpers; import tech.pegasys.pantheon.consensus.ibft.IbftProtocolSchedule; import tech.pegasys.pantheon.consensus.ibft.RoundTimer; @@ -64,6 +66,7 @@ import java.time.Instant; import java.time.ZoneId; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; import java.util.Optional; @@ -252,6 +255,9 @@ private static ControllerAndState createControllerAndFinalState( final MessageValidatorFactory messageValidatorFactory = new MessageValidatorFactory(proposerSelector, blockHeaderValidator, protocolContext); + // Disable Gossiping for integration tests. + final IbftGossip gossiper = mock(IbftGossip.class); + final IbftController ibftController = new IbftController( blockChain, @@ -260,7 +266,9 @@ private static ControllerAndState createControllerAndFinalState( finalState, new IbftRoundFactory(finalState, protocolContext, protocolSchedule), messageValidatorFactory, - protocolContext)); + protocolContext), + new HashMap<>(), + gossiper); //////////////////////////// END IBFT PantheonController //////////////////////////// return new ControllerAndState(ibftController, finalState); diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/ValidatorPeer.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/ValidatorPeer.java index ccd2cc4180..74260ef007 100644 --- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/ValidatorPeer.java +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/ValidatorPeer.java @@ -15,11 +15,11 @@ import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; import tech.pegasys.pantheon.consensus.ibft.ibftevent.IbftEvents; import tech.pegasys.pantheon.consensus.ibft.ibftevent.IbftReceivedMessageEvent; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessage; -import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessage; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.CommitPayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.MessageFactory; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.NewRoundPayload; @@ -37,6 +37,7 @@ import tech.pegasys.pantheon.ethereum.core.Block; import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; +import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; import tech.pegasys.pantheon.ethereum.p2p.wire.DefaultMessage; import java.util.Collections; @@ -51,6 +52,7 @@ public class ValidatorPeer { private final Address nodeAddress; private final KeyPair nodeKeys; private final MessageFactory messageFactory; + private final PeerConnection peerConnection = new StubbedPeerConnection(); private List receivedMessages = Lists.newArrayList(); private final IbftController localNodeController; @@ -65,11 +67,16 @@ public ValidatorPeer( this.localNodeController = localNodeController; } + public Address getNodeAddress() { + return nodeAddress; + } + public SignedData injectProposal( final ConsensusRoundIdentifier rId, final Block block) { final SignedData payload = messageFactory.createSignedProposalPayload(rId, block); - injectMessage(ProposalMessage.create(payload)); + + injectMessage(ProposalMessageData.create(payload)); return payload; } @@ -77,7 +84,7 @@ public SignedData injectPrepare( final ConsensusRoundIdentifier rId, final Hash digest) { final SignedData payload = messageFactory.createSignedPreparePayload(rId, digest); - injectMessage(PrepareMessage.create(payload)); + injectMessage(PrepareMessageData.create(payload)); return payload; } @@ -86,7 +93,7 @@ public SignedData injectCommit( final Signature commitSeal = SECP256K1.sign(digest, nodeKeys); final SignedData payload = messageFactory.createSignedCommitPayload(rId, digest, commitSeal); - injectMessage(CommitMessage.create(payload)); + injectMessage(CommitMessageData.create(payload)); return payload; } @@ -97,7 +104,7 @@ public SignedData injectNewRound( final SignedData payload = messageFactory.createSignedNewRoundPayload(rId, roundChangeCertificate, proposalPayload); - injectMessage(NewRoundMessage.create(payload)); + injectMessage(NewRoundMessageData.create(payload)); return payload; } @@ -105,7 +112,7 @@ public SignedData injectRoundChange( final ConsensusRoundIdentifier rId, final Optional preparedCertificate) { final SignedData payload = messageFactory.createSignedRoundChangePayload(rId, preparedCertificate); - injectMessage(RoundChangeMessage.create(payload)); + injectMessage(RoundChangeMessageData.create(payload)); return payload; } @@ -122,7 +129,7 @@ public void clearReceivedMessages() { } public void injectMessage(final MessageData msgData) { - final DefaultMessage message = new DefaultMessage(null, msgData); + final DefaultMessage message = new DefaultMessage(peerConnection, msgData); localNodeController.handleMessageEvent( (IbftReceivedMessageEvent) IbftEvents.fromMessage(message)); } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java index 1adefe349d..00537287f5 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java @@ -23,7 +23,7 @@ import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangePayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; -import tech.pegasys.pantheon.consensus.ibft.network.IbftNetworkPeers; +import tech.pegasys.pantheon.consensus.ibft.network.IbftMulticaster; import tech.pegasys.pantheon.crypto.SECP256K1.Signature; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; @@ -38,7 +38,7 @@ /** Class responsible for rebroadcasting IBFT messages to known validators */ public class IbftGossip { - private final IbftNetworkPeers peers; + private final IbftMulticaster peers; // Size of the seenMessages cache, should end up utilising 65bytes * this number + some meta data private final int maxSeenMessages; @@ -53,7 +53,7 @@ protected boolean removeEldestEntry(final Map.Entry eldest) } }); - IbftGossip(final IbftNetworkPeers peers, final int maxSeenMessages) { + IbftGossip(final IbftMulticaster peers, final int maxSeenMessages) { this.maxSeenMessages = maxSeenMessages; this.peers = peers; } @@ -63,7 +63,7 @@ protected boolean removeEldestEntry(final Map.Entry eldest) * * @param peers The always up to date set of connected peers that understand IBFT */ - public IbftGossip(final IbftNetworkPeers peers) { + public IbftGossip(final IbftMulticaster peers) { this(peers, 10_000); } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftMulticaster.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftMulticaster.java index bdb8350a44..4e61274f86 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftMulticaster.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftMulticaster.java @@ -12,9 +12,15 @@ */ package tech.pegasys.pantheon.consensus.ibft.network; +import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; +import java.util.Collection; + public interface IbftMulticaster { void multicastToValidators(final MessageData message); + + void multicastToValidatorsExcept( + final MessageData message, final Collection
exceptAddresses); } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java index c757f5573d..4482e68d43 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/network/IbftNetworkPeers.java @@ -56,6 +56,7 @@ public void multicastToValidators(final MessageData message) { sendMessageToSpecificAddresses(validators, message); } + @Override public void multicastToValidatorsExcept( final MessageData message, final Collection
exceptAddresses) { final Collection
includedValidators = diff --git 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 index 12a63ad7a4..d50e7234ac 100644 --- 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 @@ -67,7 +67,7 @@ public IbftController( } @VisibleForTesting - IbftController( + public IbftController( final Blockchain blockchain, final IbftFinalState ibftFinalState, final IbftBlockHeightManagerFactory ibftBlockHeightManagerFactory, diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java b/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java index 100870cb51..e941210634 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java @@ -26,6 +26,7 @@ import tech.pegasys.pantheon.consensus.ibft.IbftBlockInterface; import tech.pegasys.pantheon.consensus.ibft.IbftContext; import tech.pegasys.pantheon.consensus.ibft.IbftEventQueue; +import tech.pegasys.pantheon.consensus.ibft.IbftGossip; import tech.pegasys.pantheon.consensus.ibft.IbftProcessor; import tech.pegasys.pantheon.consensus.ibft.IbftProtocolSchedule; import tech.pegasys.pantheon.consensus.ibft.RoundTimer; @@ -77,6 +78,7 @@ import java.io.IOException; import java.time.Clock; import java.util.Collection; +import java.util.HashMap; import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -236,7 +238,9 @@ public static PantheonController init( finalState, new IbftRoundFactory(finalState, protocolContext, protocolSchedule), messageValidatorFactory, - protocolContext)); + protocolContext), + new HashMap<>(), + new IbftGossip(peers)); final IbftProcessor ibftProcessor = new IbftProcessor(ibftEventQueue, ibftController); final ExecutorService processorExecutor = Executors.newSingleThreadExecutor(); From 434433872aad80faff183c921b5d56cf4a879982 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Wed, 9 Jan 2019 13:07:10 +1000 Subject: [PATCH 33/33] [NC-1909] rewrote gossiper to take whole messages --- .../pantheon/consensus/ibft/IbftGossip.java | 122 ++++-------- .../ibft/statemachine/IbftController.java | 22 +-- .../consensus/ibft/IbftGossipTest.java | 178 ++++++------------ .../ibft/network/MockPeerFactory.java | 16 +- .../ibft/statemachine/IbftControllerTest.java | 37 ++-- .../controller/IbftPantheonController.java | 6 +- 6 files changed, 126 insertions(+), 255 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java index 00537287f5..f135feaf06 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftGossip.java @@ -13,19 +13,16 @@ package tech.pegasys.pantheon.consensus.ibft; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.CommitMessageData; +import tech.pegasys.pantheon.consensus.ibft.ibftmessage.IbftV2; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.NewRoundMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.PrepareMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.ProposalMessageData; import tech.pegasys.pantheon.consensus.ibft.ibftmessage.RoundChangeMessageData; -import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.CommitPayload; -import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.NewRoundPayload; -import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparePayload; -import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload; -import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangePayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; import tech.pegasys.pantheon.consensus.ibft.network.IbftMulticaster; import tech.pegasys.pantheon.crypto.SECP256K1.Signature; import tech.pegasys.pantheon.ethereum.core.Address; +import tech.pegasys.pantheon.ethereum.p2p.api.Message; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; import java.util.Collections; @@ -68,96 +65,41 @@ public IbftGossip(final IbftMulticaster peers) { } /** - * Retransmit a given proposal payload to all IBFT nodes + * Retransmit a given IBFT message to other known validators nodes * - * @param payload The signed payload to retransmit - * @param excludeAddresses A list of addresses to not communicate this to. Does not need to - * include the payload signer, that is handled implicitly - * @return Boolean of whether the message was rebroadcast or ignored as it's already been - * broadcast in recent memory + * @param message The raw message to be gossiped + * @return Whether the message was rebroadcast or has been ignored as a repeat */ - public boolean gossipProposalPayload( - final SignedData payload, final Address... excludeAddresses) { - final ProposalMessageData messageData = ProposalMessageData.create(payload); - return gossipMessage( - payload.getSignature(), messageData, payload.getSender(), excludeAddresses); - } - - /** - * Retransmit a given prepare payload to all IBFT nodes - * - * @param payload The signed payload to retransmit - * @param excludeAddresses A list of addresses to not communicate this to. Does not need to - * include the payload signer, that is handled implicitly - * @return Boolean of whether the message was rebroadcast or ignored as it's already been - * broadcast in recent memory - */ - public boolean gossipPreparePayload( - final SignedData payload, final Address... excludeAddresses) { - final PrepareMessageData messageData = PrepareMessageData.create(payload); - return gossipMessage( - payload.getSignature(), messageData, payload.getSender(), excludeAddresses); - } - - /** - * Retransmit a given commit payload to all IBFT nodes - * - * @param payload The signed payload to retransmit - * @param excludeAddresses A list of addresses to not communicate this to. Does not need to - * include the payload signer, that is handled implicitly - * @return Boolean of whether the message was rebroadcast or ignored as it's already been - * broadcast in recent memory - */ - public boolean gossipCommitPayload( - final SignedData payload, final Address... excludeAddresses) { - final CommitMessageData messageData = CommitMessageData.create(payload); - return gossipMessage( - payload.getSignature(), messageData, payload.getSender(), excludeAddresses); - } - - /** - * Retransmit a given round change payload to all IBFT nodes - * - * @param payload The signed payload to retransmit - * @param excludeAddresses A list of addresses to not communicate this to. Does not need to - * include the payload signer, that is handled implicitly - * @return Boolean of whether the message was rebroadcast or ignored as it's already been - * broadcast in recent memory - */ - public boolean gossipRoundChangePayload( - final SignedData payload, final Address... excludeAddresses) { - final RoundChangeMessageData messageData = RoundChangeMessageData.create(payload); - return gossipMessage( - payload.getSignature(), messageData, payload.getSender(), excludeAddresses); - } - - /** - * Retransmit a given new round payload to all IBFT nodes - * - * @param payload The signed payload to retransmit - * @param excludeAddresses A list of addresses to not communicate this to. Does not need to - * include the payload signer, that is handled implicitly - * @return Boolean of whether the message was rebroadcast or ignored as it's already been - * broadcast in recent memory - */ - public boolean gossipNewRoundPayload( - final SignedData payload, final Address... excludeAddresses) { - final NewRoundMessageData messageData = NewRoundMessageData.create(payload); - return gossipMessage( - payload.getSignature(), messageData, payload.getSender(), excludeAddresses); - } - - // Checks whether a payloads signature has been seen recently, if not it performs the retransmit - private boolean gossipMessage( - final Signature signature, - final MessageData messageData, - final Address payloadSigner, - final Address... excludeAddresses) { + public boolean gossipMessage(final Message message) { + final MessageData messageData = message.getData(); + final SignedData signedData; + switch (messageData.getCode()) { + case IbftV2.PROPOSAL: + signedData = ProposalMessageData.fromMessageData(messageData).decode(); + break; + case IbftV2.PREPARE: + signedData = PrepareMessageData.fromMessageData(messageData).decode(); + break; + case IbftV2.COMMIT: + signedData = CommitMessageData.fromMessageData(messageData).decode(); + break; + case IbftV2.ROUND_CHANGE: + signedData = RoundChangeMessageData.fromMessageData(messageData).decode(); + break; + case IbftV2.NEW_ROUND: + signedData = NewRoundMessageData.fromMessageData(messageData).decode(); + break; + default: + throw new IllegalArgumentException( + "Received message does not conform to any recognised IBFT message structure."); + } + final Signature signature = signedData.getSignature(); if (seenMessages.contains(signature)) { return false; } else { - final List
excludeAddressesList = Lists.newArrayList(excludeAddresses); - excludeAddressesList.add(payloadSigner); + final List
excludeAddressesList = + Lists.newArrayList( + message.getConnection().getPeer().getAddress(), signedData.getSender()); peers.multicastToValidatorsExcept(messageData, excludeAddressesList); seenMessages.add(signature); return true; diff --git 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 index d50e7234ac..2f616a4059 100644 --- 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 @@ -29,14 +29,12 @@ import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.Payload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; import tech.pegasys.pantheon.ethereum.chain.Blockchain; -import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.p2p.api.Message; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; import java.util.List; import java.util.Map; -import java.util.function.BiConsumer; import java.util.function.Consumer; import com.google.common.annotations.VisibleForTesting; @@ -92,42 +90,37 @@ private void handleMessage(final Message message) { final MessageData messageData = message.getData(); switch (messageData.getCode()) { case IbftV2.PROPOSAL: - processMessage( + consumeMessage( message, ProposalMessageData.fromMessageData(messageData).decode(), - gossiper::gossipProposalPayload, currentHeightManager::handleProposalPayload); break; case IbftV2.PREPARE: - processMessage( + consumeMessage( message, PrepareMessageData.fromMessageData(messageData).decode(), - gossiper::gossipPreparePayload, currentHeightManager::handlePreparePayload); break; case IbftV2.COMMIT: - processMessage( + consumeMessage( message, CommitMessageData.fromMessageData(messageData).decode(), - gossiper::gossipCommitPayload, currentHeightManager::handleCommitPayload); break; case IbftV2.ROUND_CHANGE: - processMessage( + consumeMessage( message, RoundChangeMessageData.fromMessageData(messageData).decode(), - gossiper::gossipRoundChangePayload, currentHeightManager::handleRoundChangePayload); break; case IbftV2.NEW_ROUND: - processMessage( + consumeMessage( message, NewRoundMessageData.fromMessageData(messageData).decode(), - gossiper::gossipNewRoundPayload, currentHeightManager::handleNewRoundPayload); break; @@ -137,13 +130,12 @@ private void handleMessage(final Message message) { } } - private

void processMessage( + private

void consumeMessage( final Message message, final SignedData

signedPayload, - final BiConsumer, Address> gossipMessage, final Consumer> handleMessage) { if (processMessage(signedPayload, message)) { - gossipMessage.accept(signedPayload, message.getConnection().getPeer().getAddress()); + gossiper.gossipMessage(message); handleMessage.accept(signedPayload); } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java index 7c90e6cfd7..b1b366444c 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftGossipTest.java @@ -26,12 +26,15 @@ import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; import tech.pegasys.pantheon.consensus.ibft.network.IbftNetworkPeers; +import tech.pegasys.pantheon.consensus.ibft.network.MockPeerFactory; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.ethereum.core.Address; -import tech.pegasys.pantheon.ethereum.core.Util; +import tech.pegasys.pantheon.ethereum.core.AddressHelpers; +import tech.pegasys.pantheon.ethereum.p2p.api.Message; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; +import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; +import tech.pegasys.pantheon.ethereum.p2p.wire.DefaultMessage; -import java.util.function.BiFunction; import java.util.function.Function; import org.junit.Before; @@ -44,173 +47,101 @@ public class IbftGossipTest { private IbftGossip ibftGossip; @Mock private IbftNetworkPeers ibftNetworkPeers; + private PeerConnection peerConnection; + private static final Address senderAddress = AddressHelpers.ofValue(9); @Before public void setup() { ibftGossip = new IbftGossip(ibftNetworkPeers, 10); + peerConnection = MockPeerFactory.create(senderAddress); } - private

void assertRebroadcastToAllExceptSigner( + private

void assertRebroadcastToAllExceptSignerAndSender( final Function> createPayload, - final Function, MessageData> createMessageData, - final Function, Boolean> gossiper) { + final Function, MessageData> createMessageData) { final KeyPair keypair = KeyPair.generate(); final SignedData

payload = createPayload.apply(keypair); - final boolean gossipResult = gossiper.apply(payload); - assertThat(gossipResult).isTrue(); final MessageData messageData = createMessageData.apply(payload); - verify(ibftNetworkPeers) - .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); - } + final Message message = new DefaultMessage(peerConnection, messageData); - private

void assertRebroadcastToAllExceptSignerAndSuppliedExceptions( - final Function> createPayload, - final Function, MessageData> createMessageData, - final BiFunction, Address, Boolean> gossiper) { - final KeyPair keypair = KeyPair.generate(); - final KeyPair keypair2 = KeyPair.generate(); - final Address key2Address = Util.publicKeyToAddress(keypair2.getPublicKey()); - final SignedData

payload = createPayload.apply(keypair); - final boolean gossipResult = gossiper.apply(payload, key2Address); + final boolean gossipResult = ibftGossip.gossipMessage(message); assertThat(gossipResult).isTrue(); - final MessageData messageData = createMessageData.apply(payload); verify(ibftNetworkPeers) - .multicastToValidatorsExcept(messageData, newArrayList(key2Address, payload.getSender())); + .multicastToValidatorsExcept(messageData, newArrayList(senderAddress, payload.getSender())); } private

void assertRebroadcastOnlyOnce( final Function> createPayload, - final Function, MessageData> createMessageData, - final Function, Boolean> gossiper) { + final Function, MessageData> createMessageData) { final KeyPair keypair = KeyPair.generate(); final SignedData

payload = createPayload.apply(keypair); - final boolean gossip1Result = gossiper.apply(payload); - final boolean gossip2Result = gossiper.apply(payload); + final MessageData messageData = createMessageData.apply(payload); + final Message message = new DefaultMessage(peerConnection, messageData); + + final boolean gossip1Result = ibftGossip.gossipMessage(message); + final boolean gossip2Result = ibftGossip.gossipMessage(message); assertThat(gossip1Result).isTrue(); assertThat(gossip2Result).isFalse(); - final MessageData messageData = createMessageData.apply(payload); verify(ibftNetworkPeers, times(1)) - .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + .multicastToValidatorsExcept(messageData, newArrayList(senderAddress, payload.getSender())); } @Test - public void assertRebroadcastsProposalToAllExceptSigner() { - assertRebroadcastToAllExceptSigner( - TestHelpers::createSignedProposalPayload, - ProposalMessageData::create, - ibftGossip::gossipProposalPayload); - } - - @Test - public void assertRebroadcastsProposalToAllExceptSignerAndSuppliedExceptions() { - assertRebroadcastToAllExceptSignerAndSuppliedExceptions( - TestHelpers::createSignedProposalPayload, - ProposalMessageData::create, - ibftGossip::gossipProposalPayload); + public void assertRebroadcastsProposalToAllExceptSignerAndSender() { + assertRebroadcastToAllExceptSignerAndSender( + TestHelpers::createSignedProposalPayload, ProposalMessageData::create); } @Test public void assertRebroadcastsProposalOnlyOnce() { assertRebroadcastOnlyOnce( - TestHelpers::createSignedProposalPayload, - ProposalMessageData::create, - ibftGossip::gossipProposalPayload); + TestHelpers::createSignedProposalPayload, ProposalMessageData::create); } @Test - public void assertRebroadcastsPrepareToAllExceptSigner() { - assertRebroadcastToAllExceptSigner( - TestHelpers::createSignedPreparePayload, - PrepareMessageData::create, - ibftGossip::gossipPreparePayload); - } - - @Test - public void assertRebroadcastsPrepareToAllExceptSignerAndSuppliedExceptions() { - assertRebroadcastToAllExceptSignerAndSuppliedExceptions( - TestHelpers::createSignedPreparePayload, - PrepareMessageData::create, - ibftGossip::gossipPreparePayload); + public void assertRebroadcastsPrepareToAllExceptSignerAndSender() { + assertRebroadcastToAllExceptSignerAndSender( + TestHelpers::createSignedPreparePayload, PrepareMessageData::create); } @Test public void assertRebroadcastsPrepareOnlyOnce() { - assertRebroadcastOnlyOnce( - TestHelpers::createSignedPreparePayload, - PrepareMessageData::create, - ibftGossip::gossipPreparePayload); + assertRebroadcastOnlyOnce(TestHelpers::createSignedPreparePayload, PrepareMessageData::create); } @Test - public void assertRebroadcastsCommitToAllExceptSigner() { - assertRebroadcastToAllExceptSigner( - TestHelpers::createSignedCommitPayload, - CommitMessageData::create, - ibftGossip::gossipCommitPayload); - } - - @Test - public void assertRebroadcastsCommitToAllExceptSignerAndSuppliedExceptions() { - assertRebroadcastToAllExceptSignerAndSuppliedExceptions( - TestHelpers::createSignedCommitPayload, - CommitMessageData::create, - ibftGossip::gossipCommitPayload); + public void assertRebroadcastsCommitToAllExceptSignerAndSender() { + assertRebroadcastToAllExceptSignerAndSender( + TestHelpers::createSignedCommitPayload, CommitMessageData::create); } @Test public void assertRebroadcastsCommitOnlyOnce() { - assertRebroadcastOnlyOnce( - TestHelpers::createSignedCommitPayload, - CommitMessageData::create, - ibftGossip::gossipCommitPayload); - } - - @Test - public void assertRebroadcastsRoundChangeToAllExceptSigner() { - assertRebroadcastToAllExceptSigner( - TestHelpers::createSignedRoundChangePayload, - RoundChangeMessageData::create, - ibftGossip::gossipRoundChangePayload); + assertRebroadcastOnlyOnce(TestHelpers::createSignedCommitPayload, CommitMessageData::create); } @Test - public void assertRebroadcastsRoundChangeToAllExceptSignerAndSuppliedExceptions() { - assertRebroadcastToAllExceptSignerAndSuppliedExceptions( - TestHelpers::createSignedRoundChangePayload, - RoundChangeMessageData::create, - ibftGossip::gossipRoundChangePayload); + public void assertRebroadcastsRoundChangeToAllExceptSignerAndSender() { + assertRebroadcastToAllExceptSignerAndSender( + TestHelpers::createSignedRoundChangePayload, RoundChangeMessageData::create); } @Test public void assertRebroadcastsRoundChangeOnlyOnce() { assertRebroadcastOnlyOnce( - TestHelpers::createSignedRoundChangePayload, - RoundChangeMessageData::create, - ibftGossip::gossipRoundChangePayload); + TestHelpers::createSignedRoundChangePayload, RoundChangeMessageData::create); } @Test - public void assertRebroadcastsNewRoundToAllExceptSigner() { - assertRebroadcastToAllExceptSigner( - TestHelpers::createSignedNewRoundPayload, - NewRoundMessageData::create, - ibftGossip::gossipNewRoundPayload); - } - - @Test - public void assertRebroadcastsNewRoundToAllExceptSignerAndSuppliedExceptions() { - assertRebroadcastToAllExceptSignerAndSuppliedExceptions( - TestHelpers::createSignedNewRoundPayload, - NewRoundMessageData::create, - ibftGossip::gossipNewRoundPayload); + public void assertRebroadcastsNewRoundToAllExceptSignerAndSender() { + assertRebroadcastToAllExceptSignerAndSender( + TestHelpers::createSignedNewRoundPayload, NewRoundMessageData::create); } @Test public void assertRebroadcastsNewRoundOnlyOnce() { assertRebroadcastOnlyOnce( - TestHelpers::createSignedNewRoundPayload, - NewRoundMessageData::create, - ibftGossip::gossipNewRoundPayload); + TestHelpers::createSignedNewRoundPayload, NewRoundMessageData::create); } @Test @@ -218,41 +149,46 @@ public void evictMessageRecordAtCapacity() { final KeyPair keypair = KeyPair.generate(); final SignedData payload = TestHelpers.createSignedProposalPayloadWithRound(keypair, 0); - final boolean gossip1Result = ibftGossip.gossipProposalPayload(payload); - final boolean gossip2Result = ibftGossip.gossipProposalPayload(payload); + final MessageData messageData = ProposalMessageData.create(payload); + final Message message = new DefaultMessage(peerConnection, messageData); + final boolean gossip1Result = ibftGossip.gossipMessage(message); + final boolean gossip2Result = ibftGossip.gossipMessage(message); assertThat(gossip1Result).isTrue(); assertThat(gossip2Result).isFalse(); - final MessageData messageData = ProposalMessageData.create(payload); verify(ibftNetworkPeers, times(1)) - .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + .multicastToValidatorsExcept(messageData, newArrayList(senderAddress, payload.getSender())); for (int i = 1; i <= 9; i++) { final SignedData nextPayload = TestHelpers.createSignedProposalPayloadWithRound(keypair, i); - final boolean nextGossipResult = ibftGossip.gossipProposalPayload(nextPayload); + final MessageData nextMessageData = ProposalMessageData.create(nextPayload); + final Message nextMessage = new DefaultMessage(peerConnection, nextMessageData); + final boolean nextGossipResult = ibftGossip.gossipMessage(nextMessage); assertThat(nextGossipResult).isTrue(); } - final boolean gossip3Result = ibftGossip.gossipProposalPayload(payload); + final boolean gossip3Result = ibftGossip.gossipMessage(message); assertThat(gossip3Result).isFalse(); verify(ibftNetworkPeers, times(1)) - .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + .multicastToValidatorsExcept(messageData, newArrayList(senderAddress, payload.getSender())); { final SignedData nextPayload = TestHelpers.createSignedProposalPayloadWithRound(keypair, 10); - final boolean nextGossipResult = ibftGossip.gossipProposalPayload(nextPayload); + final MessageData nextMessageData = ProposalMessageData.create(nextPayload); + final Message nextMessage = new DefaultMessage(peerConnection, nextMessageData); + final boolean nextGossipResult = ibftGossip.gossipMessage(nextMessage); assertThat(nextGossipResult).isTrue(); } - final boolean gossip4Result = ibftGossip.gossipProposalPayload(payload); + final boolean gossip4Result = ibftGossip.gossipMessage(message); assertThat(gossip4Result).isTrue(); verify(ibftNetworkPeers, times(2)) - .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + .multicastToValidatorsExcept(messageData, newArrayList(senderAddress, payload.getSender())); - final boolean gossip5Result = ibftGossip.gossipProposalPayload(payload); + final boolean gossip5Result = ibftGossip.gossipMessage(message); assertThat(gossip5Result).isFalse(); verify(ibftNetworkPeers, times(2)) - .multicastToValidatorsExcept(messageData, newArrayList(payload.getSender())); + .multicastToValidatorsExcept(messageData, newArrayList(senderAddress, payload.getSender())); } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/MockPeerFactory.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/MockPeerFactory.java index 559271ddd1..150e47b5a1 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/MockPeerFactory.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/network/MockPeerFactory.java @@ -15,20 +15,26 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import tech.pegasys.pantheon.ethereum.core.Address; +import tech.pegasys.pantheon.ethereum.core.AddressHelpers; import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; import tech.pegasys.pantheon.ethereum.p2p.wire.PeerInfo; public class MockPeerFactory { public static PeerConnection create() { - PeerConnection peerConnection = mock(PeerConnection.class); - PeerInfo peerInfo = createPeerInfo(); + return create(AddressHelpers.ofValue(9)); + } + + public static PeerConnection create(final Address address) { + final PeerConnection peerConnection = mock(PeerConnection.class); + final PeerInfo peerInfo = createPeerInfo(address); when(peerConnection.getPeer()).thenReturn(peerInfo); return peerConnection; } - public static PeerInfo createPeerInfo() { - PeerInfo peerInfo = mock(PeerInfo.class); - when(peerInfo.getAddress()).thenReturn(null); + public static PeerInfo createPeerInfo(final Address address) { + final PeerInfo peerInfo = mock(PeerInfo.class); + when(peerInfo.getAddress()).thenReturn(address); return peerInfo; } } diff --git 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 index 79b6827b98..85ecdd4b08 100644 --- 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 @@ -39,7 +39,6 @@ import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangePayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; -import tech.pegasys.pantheon.consensus.ibft.network.MockPeerFactory; import tech.pegasys.pantheon.ethereum.chain.Blockchain; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; @@ -140,11 +139,11 @@ public void startsNewBlockHeightManagerAndReplaysFutureMessages() { verify(blockHeightManager).start(); verify(blockHeightManager, never()).handleProposalPayload(signedProposal); verify(blockHeightManager).handlePreparePayload(signedPrepare); - verify(ibftGossip).gossipPreparePayload(signedPrepare, (Address) null); + verify(ibftGossip).gossipMessage(prepareMessage); verify(blockHeightManager).handleCommitPayload(signedCommit); - verify(ibftGossip).gossipCommitPayload(signedCommit, (Address) null); + verify(ibftGossip).gossipMessage(commitMessage); verify(blockHeightManager).handleRoundChangePayload(signedRoundChange); - verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, (Address) null); + verify(ibftGossip).gossipMessage(roundChangeMessage); verify(blockHeightManager, never()).handleNewRoundPayload(signedNewRound); } @@ -169,15 +168,15 @@ public void createsNewBlockHeightManagerAndReplaysFutureMessagesOnNewChainHeadEv verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verify(blockHeightManager).handleProposalPayload(signedProposal); - verify(ibftGossip).gossipProposalPayload(signedProposal, (Address) null); + verify(ibftGossip).gossipMessage(proposalMessage); verify(blockHeightManager).handlePreparePayload(signedPrepare); - verify(ibftGossip).gossipPreparePayload(signedPrepare, (Address) null); + verify(ibftGossip).gossipMessage(prepareMessage); verify(blockHeightManager).handleCommitPayload(signedCommit); - verify(ibftGossip).gossipCommitPayload(signedCommit, (Address) null); + verify(ibftGossip).gossipMessage(commitMessage); verify(blockHeightManager).handleRoundChangePayload(signedRoundChange); - verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, (Address) null); + verify(ibftGossip).gossipMessage(roundChangeMessage); verify(blockHeightManager).handleNewRoundPayload(signedNewRound); - verify(ibftGossip).gossipNewRoundPayload(signedNewRound, (Address) null); + verify(ibftGossip).gossipMessage(newRoundMessage); } @Test @@ -208,7 +207,7 @@ public void proposalForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handleProposalPayload(signedProposal); - verify(ibftGossip).gossipProposalPayload(signedProposal, (Address) null); + verify(ibftGossip).gossipMessage(proposalMessage); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -222,7 +221,7 @@ public void prepareForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handlePreparePayload(signedPrepare); - verify(ibftGossip).gossipPreparePayload(signedPrepare, (Address) null); + verify(ibftGossip).gossipMessage(prepareMessage); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -236,7 +235,7 @@ public void commitForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handleCommitPayload(signedCommit); - verify(ibftGossip).gossipCommitPayload(signedCommit, (Address) null); + verify(ibftGossip).gossipMessage(commitMessage); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -251,7 +250,7 @@ public void newRoundForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handleNewRoundPayload(signedNewRound); - verify(ibftGossip).gossipNewRoundPayload(signedNewRound, (Address) null); + verify(ibftGossip).gossipMessage(newRoundMessage); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -266,7 +265,7 @@ public void roundChangeForCurrentHeightIsPassedToBlockHeightManager() { assertThat(futureMessages).isEmpty(); verify(blockHeightManager).handleRoundChangePayload(signedRoundChange); - verify(ibftGossip).gossipRoundChangePayload(signedRoundChange, (Address) null); + verify(ibftGossip).gossipMessage(roundChangeMessage); verify(blockHeightManager, atLeastOnce()).getChainHeight(); verify(blockHeightManager).start(); verifyNoMoreInteractions(blockHeightManager); @@ -426,7 +425,7 @@ private void setupProposal( when(proposalPayload.getRoundIdentifier()).thenReturn(roundIdentifier); when(proposalMessageData.getCode()).thenReturn(IbftV2.PROPOSAL); when(proposalMessageData.decode()).thenReturn(signedProposal); - proposalMessage = new DefaultMessage(MockPeerFactory.create(), proposalMessageData); + proposalMessage = new DefaultMessage(null, proposalMessageData); } private void setupPrepare( @@ -436,7 +435,7 @@ private void setupPrepare( when(preparePayload.getRoundIdentifier()).thenReturn(roundIdentifier); when(prepareMessageData.getCode()).thenReturn(IbftV2.PREPARE); when(prepareMessageData.decode()).thenReturn(signedPrepare); - prepareMessage = new DefaultMessage(MockPeerFactory.create(), prepareMessageData); + prepareMessage = new DefaultMessage(null, prepareMessageData); } private void setupCommit( @@ -446,7 +445,7 @@ private void setupCommit( when(commitPayload.getRoundIdentifier()).thenReturn(roundIdentifier); when(commitMessageData.getCode()).thenReturn(IbftV2.COMMIT); when(commitMessageData.decode()).thenReturn(signedCommit); - commitMessage = new DefaultMessage(MockPeerFactory.create(), commitMessageData); + commitMessage = new DefaultMessage(null, commitMessageData); } private void setupNewRound( @@ -456,7 +455,7 @@ private void setupNewRound( when(newRoundPayload.getRoundIdentifier()).thenReturn(roundIdentifier); when(newRoundMessageData.getCode()).thenReturn(IbftV2.NEW_ROUND); when(newRoundMessageData.decode()).thenReturn(signedNewRound); - newRoundMessage = new DefaultMessage(MockPeerFactory.create(), newRoundMessageData); + newRoundMessage = new DefaultMessage(null, newRoundMessageData); } private void setupRoundChange( @@ -466,6 +465,6 @@ private void setupRoundChange( when(roundChangePayload.getRoundIdentifier()).thenReturn(roundIdentifier); when(roundChangeMessageData.getCode()).thenReturn(IbftV2.ROUND_CHANGE); when(roundChangeMessageData.decode()).thenReturn(signedRoundChange); - roundChangeMessage = new DefaultMessage(MockPeerFactory.create(), roundChangeMessageData); + roundChangeMessage = new DefaultMessage(null, roundChangeMessageData); } } diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java b/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java index e941210634..100870cb51 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java @@ -26,7 +26,6 @@ import tech.pegasys.pantheon.consensus.ibft.IbftBlockInterface; import tech.pegasys.pantheon.consensus.ibft.IbftContext; import tech.pegasys.pantheon.consensus.ibft.IbftEventQueue; -import tech.pegasys.pantheon.consensus.ibft.IbftGossip; import tech.pegasys.pantheon.consensus.ibft.IbftProcessor; import tech.pegasys.pantheon.consensus.ibft.IbftProtocolSchedule; import tech.pegasys.pantheon.consensus.ibft.RoundTimer; @@ -78,7 +77,6 @@ import java.io.IOException; import java.time.Clock; import java.util.Collection; -import java.util.HashMap; import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -238,9 +236,7 @@ public static PantheonController init( finalState, new IbftRoundFactory(finalState, protocolContext, protocolSchedule), messageValidatorFactory, - protocolContext), - new HashMap<>(), - new IbftGossip(peers)); + protocolContext)); final IbftProcessor ibftProcessor = new IbftProcessor(ibftEventQueue, ibftController); final ExecutorService processorExecutor = Executors.newSingleThreadExecutor();