From 698e7823d51efa76098b7d2d2ea5df29f9a0f614 Mon Sep 17 00:00:00 2001 From: tmohay <37158202+rain-on@users.noreply.github.com> Date: Fri, 14 Dec 2018 16:58:54 +1100 Subject: [PATCH] Remove Ibft's InRoundPayload interface (#423) An abstraction existed which allowed for the differentiation of IBFT messages which were used 'in-round' (Proposal, Prepare, Commit) and those that were inter-round (RoundChange, NewRound). This abstraction has not proven useful now that actual capability has been developed, and as such is being removed. --- .../ibft/ibftmessagedata/CommitPayload.java | 2 +- .../ibft/ibftmessagedata/InRoundPayload.java | 19 ------------------- .../ibft/ibftmessagedata/NewRoundPayload.java | 3 ++- .../ibft/ibftmessagedata/Payload.java | 3 +++ .../ibft/ibftmessagedata/PreparePayload.java | 2 +- .../ibft/ibftmessagedata/ProposalPayload.java | 2 +- .../ibftmessagedata/RoundChangePayload.java | 3 ++- .../ibft/statemachine/RoundChangeManager.java | 2 +- .../ibft/validation/MessageValidator.java | 4 ++-- .../validation/NewRoundMessageValidator.java | 8 ++++---- .../RoundChangeMessageValidator.java | 2 +- .../ibftmessagedata/NewRoundPayloadTest.java | 4 ++-- .../RoundChangeCertificateTest.java | 4 ++-- .../RoundChangePayloadTest.java | 6 +++--- .../NewRoundMessageValidatorTest.java | 2 +- 15 files changed, 26 insertions(+), 40 deletions(-) delete mode 100644 consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/InRoundPayload.java diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/CommitPayload.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/CommitPayload.java index b756ca7346..47562dd049 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/CommitPayload.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/CommitPayload.java @@ -22,7 +22,7 @@ import tech.pegasys.pantheon.ethereum.rlp.RLPOutput; import java.util.Objects; import java.util.StringJoiner; -public class CommitPayload implements InRoundPayload { +public class CommitPayload implements Payload { private static final int TYPE = IbftV2.COMMIT; private final ConsensusRoundIdentifier roundIdentifier; private final Hash digest; diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/InRoundPayload.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/InRoundPayload.java deleted file mode 100644 index be6ae9a8fe..0000000000 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/InRoundPayload.java +++ /dev/null @@ -1,19 +0,0 @@ -/* - * 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.ibftmessagedata; - -import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; - -public interface InRoundPayload extends Payload { - ConsensusRoundIdentifier getRoundIdentifier(); -} diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/NewRoundPayload.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/NewRoundPayload.java index d582eb2be3..76f1abcf29 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/NewRoundPayload.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/NewRoundPayload.java @@ -36,7 +36,8 @@ public class NewRoundPayload implements Payload { this.proposalPayload = proposalPayload; } - public ConsensusRoundIdentifier getRoundChangeIdentifier() { + @Override + public ConsensusRoundIdentifier getRoundIdentifier() { return roundChangeIdentifier; } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/Payload.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/Payload.java index f7291ba9e7..1c400ecf98 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/Payload.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/Payload.java @@ -12,6 +12,7 @@ */ package tech.pegasys.pantheon.consensus.ibft.ibftmessagedata; +import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.rlp.BytesValueRLPOutput; import tech.pegasys.pantheon.ethereum.rlp.RLPInput; @@ -22,6 +23,8 @@ public interface Payload { void writeTo(final RLPOutput rlpOutput); + ConsensusRoundIdentifier getRoundIdentifier(); + default BytesValue encoded() { BytesValueRLPOutput rlpOutput = new BytesValueRLPOutput(); writeTo(rlpOutput); diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/PreparePayload.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/PreparePayload.java index f550ddf904..eb90a57abf 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/PreparePayload.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/PreparePayload.java @@ -21,7 +21,7 @@ import tech.pegasys.pantheon.ethereum.rlp.RLPOutput; import java.util.Objects; import java.util.StringJoiner; -public class PreparePayload implements InRoundPayload { +public class PreparePayload implements Payload { private static final int TYPE = IbftV2.PREPARE; private final ConsensusRoundIdentifier roundIdentifier; private final Hash digest; diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/ProposalPayload.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/ProposalPayload.java index 314c092eac..8ddb36e6d9 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/ProposalPayload.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/ProposalPayload.java @@ -22,7 +22,7 @@ import tech.pegasys.pantheon.ethereum.rlp.RLPOutput; import java.util.Objects; import java.util.StringJoiner; -public class ProposalPayload implements InRoundPayload { +public class ProposalPayload implements Payload { private static final int TYPE = IbftV2.PROPOSAL; private final ConsensusRoundIdentifier roundIdentifier; private final Block block; diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/RoundChangePayload.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/RoundChangePayload.java index 7ed5dfd5e1..dcd6d1a846 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/RoundChangePayload.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/RoundChangePayload.java @@ -35,7 +35,8 @@ public class RoundChangePayload implements Payload { this.preparedCertificate = preparedCertificate; } - public ConsensusRoundIdentifier getRoundChangeIdentifier() { + @Override + public ConsensusRoundIdentifier getRoundIdentifier() { return roundChangeIdentifier; } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeManager.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeManager.java index 8649ce47d7..11a1d7a139 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeManager.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeManager.java @@ -121,7 +121,7 @@ public class RoundChangeManager { } private RoundChangeStatus storeRoundChangeMessage(final SignedData msg) { - final ConsensusRoundIdentifier msgTargetRound = msg.getPayload().getRoundChangeIdentifier(); + final ConsensusRoundIdentifier msgTargetRound = msg.getPayload().getRoundIdentifier(); final RoundChangeStatus roundChangeStatus = roundChangeCache.computeIfAbsent( diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/MessageValidator.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/MessageValidator.java index 4cc2ba3c99..061f766383 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/MessageValidator.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/MessageValidator.java @@ -18,7 +18,7 @@ import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; import tech.pegasys.pantheon.consensus.ibft.IbftContext; import tech.pegasys.pantheon.consensus.ibft.IbftExtraData; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.CommitPayload; -import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.InRoundPayload; +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.SignedData; @@ -159,7 +159,7 @@ public class MessageValidator { } private boolean isMessageForCurrentRoundFromValidatorAndProposalAvailable( - final SignedData msg, final String msgType) { + final SignedData msg, final String msgType) { if (!msg.getPayload().getRoundIdentifier().equals(roundIdentifier)) { LOG.info("Invalid {} message, does not match current round.", msgType); diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidator.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidator.java index 3c17f5e483..3635231a74 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidator.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidator.java @@ -55,7 +55,7 @@ public class NewRoundMessageValidator { public boolean validateNewRoundMessage(final SignedData msg) { final NewRoundPayload payload = msg.getPayload(); - final ConsensusRoundIdentifier rootRoundIdentifier = payload.getRoundChangeIdentifier(); + final ConsensusRoundIdentifier rootRoundIdentifier = payload.getRoundIdentifier(); final Address expectedProposer = proposerSelector.selectProposerForRound(rootRoundIdentifier); final RoundChangeCertificate roundChangeCert = payload.getRoundChangeCertificate(); @@ -64,12 +64,12 @@ public class NewRoundMessageValidator { return false; } - if (msg.getPayload().getRoundChangeIdentifier().getSequenceNumber() != chainHeight) { + if (msg.getPayload().getRoundIdentifier().getSequenceNumber() != chainHeight) { LOG.info("Invalid NewRound message, not valid for local chain height."); return false; } - if (msg.getPayload().getRoundChangeIdentifier().getRoundNumber() == 0) { + if (msg.getPayload().getRoundIdentifier().getRoundNumber() == 0) { LOG.info("Invalid NewRound message, illegally targets a new round of 0."); return false; } @@ -110,7 +110,7 @@ public class NewRoundMessageValidator { if (!roundChangeCert .getRoundChangePayloads() .stream() - .allMatch(p -> p.getPayload().getRoundChangeIdentifier().equals(expectedRound))) { + .allMatch(p -> p.getPayload().getRoundIdentifier().equals(expectedRound))) { LOG.info( "Invalid NewRound message, not all embedded RoundChange messages have a " + "matching target round."); diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/RoundChangeMessageValidator.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/RoundChangeMessageValidator.java index c26c667787..b6d33a33f2 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/RoundChangeMessageValidator.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/RoundChangeMessageValidator.java @@ -54,7 +54,7 @@ public class RoundChangeMessageValidator { return false; } - final ConsensusRoundIdentifier targetRound = msg.getPayload().getRoundChangeIdentifier(); + final ConsensusRoundIdentifier targetRound = msg.getPayload().getRoundIdentifier(); if (targetRound.getSequenceNumber() != chainHeight) { LOG.info("Invalid RoundChange message, not valid for local chain height."); diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/NewRoundPayloadTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/NewRoundPayloadTest.java index a7f13f42df..3da7d9f7fb 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/NewRoundPayloadTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/NewRoundPayloadTest.java @@ -54,7 +54,7 @@ public class NewRoundPayloadTest { final NewRoundPayload newRoundPayload = NewRoundPayload.readFrom(rlpInput); assertThat(newRoundPayload.getProposalPayload()).isEqualTo(proposalPayloadSignedData); assertThat(newRoundPayload.getRoundChangeCertificate()).isEqualTo(roundChangeCertificate); - assertThat(newRoundPayload.getRoundChangeIdentifier()).isEqualTo(ROUND_IDENTIFIER); + assertThat(newRoundPayload.getRoundIdentifier()).isEqualTo(ROUND_IDENTIFIER); assertThat(newRoundPayload.getMessageType()).isEqualTo(IbftV2.NEW_ROUND); } @@ -87,7 +87,7 @@ public class NewRoundPayloadTest { final NewRoundPayload newRoundPayload = NewRoundPayload.readFrom(rlpInput); assertThat(newRoundPayload.getProposalPayload()).isEqualTo(signedProposal); assertThat(newRoundPayload.getRoundChangeCertificate()).isEqualTo(roundChangeCertificate); - assertThat(newRoundPayload.getRoundChangeIdentifier()).isEqualTo(ROUND_IDENTIFIER); + assertThat(newRoundPayload.getRoundIdentifier()).isEqualTo(ROUND_IDENTIFIER); assertThat(newRoundPayload.getMessageType()).isEqualTo(IbftV2.NEW_ROUND); } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/RoundChangeCertificateTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/RoundChangeCertificateTest.java index 1a198b4b19..6c4580b297 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/RoundChangeCertificateTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/RoundChangeCertificateTest.java @@ -45,7 +45,7 @@ public class RoundChangeCertificateTest { RoundChangePayload actualRoundChangePayload = RoundChangePayload.readFrom(rlpInput); assertThat(actualRoundChangePayload.getPreparedCertificate()).isEqualTo(Optional.empty()); - assertThat(actualRoundChangePayload.getRoundChangeIdentifier()).isEqualTo(ROUND_IDENTIFIER); + assertThat(actualRoundChangePayload.getRoundIdentifier()).isEqualTo(ROUND_IDENTIFIER); assertThat(actualRoundChangePayload.getMessageType()).isEqualTo(IbftV2.ROUND_CHANGE); } @@ -73,7 +73,7 @@ public class RoundChangeCertificateTest { assertThat(actualRoundChangePayload.getPreparedCertificate()) .isEqualTo(Optional.of(preparedCert)); - assertThat(actualRoundChangePayload.getRoundChangeIdentifier()).isEqualTo(ROUND_IDENTIFIER); + assertThat(actualRoundChangePayload.getRoundIdentifier()).isEqualTo(ROUND_IDENTIFIER); assertThat(actualRoundChangePayload.getMessageType()).isEqualTo(IbftV2.ROUND_CHANGE); } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/RoundChangePayloadTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/RoundChangePayloadTest.java index d8dc244025..334b22da59 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/RoundChangePayloadTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/ibftmessagedata/RoundChangePayloadTest.java @@ -46,7 +46,7 @@ public class RoundChangePayloadTest { final RLPInput rlpInput = RLP.input(rlpOut.encoded()); RoundChangePayload actualRoundChangePayload = RoundChangePayload.readFrom(rlpInput); - assertThat(actualRoundChangePayload.getRoundChangeIdentifier()).isEqualTo(ROUND_IDENTIFIER); + assertThat(actualRoundChangePayload.getRoundIdentifier()).isEqualTo(ROUND_IDENTIFIER); assertThat(actualRoundChangePayload.getPreparedCertificate()).isEqualTo(Optional.empty()); assertThat(actualRoundChangePayload.getMessageType()).isEqualTo(IbftV2.ROUND_CHANGE); } @@ -65,7 +65,7 @@ public class RoundChangePayloadTest { final RLPInput rlpInput = RLP.input(rlpOut.encoded()); RoundChangePayload actualRoundChangePayload = RoundChangePayload.readFrom(rlpInput); - assertThat(actualRoundChangePayload.getRoundChangeIdentifier()).isEqualTo(ROUND_IDENTIFIER); + assertThat(actualRoundChangePayload.getRoundIdentifier()).isEqualTo(ROUND_IDENTIFIER); assertThat(actualRoundChangePayload.getPreparedCertificate()) .isEqualTo(Optional.of(preparedCertificate)); assertThat(actualRoundChangePayload.getMessageType()).isEqualTo(IbftV2.ROUND_CHANGE); @@ -88,7 +88,7 @@ public class RoundChangePayloadTest { final RLPInput rlpInput = RLP.input(rlpOut.encoded()); RoundChangePayload actualRoundChangePayload = RoundChangePayload.readFrom(rlpInput); - assertThat(actualRoundChangePayload.getRoundChangeIdentifier()).isEqualTo(ROUND_IDENTIFIER); + assertThat(actualRoundChangePayload.getRoundIdentifier()).isEqualTo(ROUND_IDENTIFIER); assertThat(actualRoundChangePayload.getPreparedCertificate()) .isEqualTo(Optional.of(preparedCert)); assertThat(actualRoundChangePayload.getMessageType()).isEqualTo(IbftV2.ROUND_CHANGE); diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidatorTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidatorTest.java index d634ebc7f4..969e79f9ab 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidatorTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidatorTest.java @@ -109,7 +109,7 @@ public class NewRoundMessageValidatorTest { final MessageFactory messageCreator = new MessageFactory(signingKey); return messageCreator.createSignedNewRoundPayload( - payload.getRoundChangeIdentifier(), + payload.getRoundIdentifier(), payload.getRoundChangeCertificate(), payload.getProposalPayload()); }