IBFT 2.1 Message rework, piggybacking blocks on msgs. #769

rain-on · 2019-02-05T02:11:57Z

No description provided.

rain-on · 2019-02-05T05:15:56Z

RoundCHangeMessageValidator does not validate block vs proposal (As per NewRound).
[Now resolved]

rain-on · 2019-02-05T05:58:07Z

Do we need to fully validate a block on reception of a RoundChange message? or only Proposal/NewRound?
Why don't we check the block number against the sequence number?

rain-on · 2019-02-05T06:02:05Z

Move Block Validation out of ProposalBlockConsistencyChecker, and put it in the Message Validator(s).
[Now Resolved]

rain-on · 2019-02-05T06:34:51Z

Can we merge *MessageData with correlated MessageWrapper?

...sus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/MessageValidator.java

consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/payload/MessageFactory.java

...ft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeArtifacts.java

.../src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidator.java

.../java/tech/pegasys/pantheon/consensus/ibft/validation/ProposalBlockConsistencyValidator.java

rain-on · 2019-02-07T03:09:33Z

...c/main/java/tech/pegasys/pantheon/consensus/ibft/validation/RoundChangeMessageValidator.java

+  private static final Logger LOG = LogManager.getLogger();
+
+  private final RoundChangePayloadValidator signedDataValidator;
+  private final ProposalBlockConsistencyValidator proposalBlockConsistencyChecker;


checker to validator.

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

jframe · 2019-02-07T05:40:21Z

...ft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeArtifacts.java

+        }
+      }
+    }
+    if (!newestRoundChange.isPresent()) {


try

Optional<Block> proposedBlock = newestRoundChange.flatMap(RoundChange::getProposedBlock); return new RoundChangeArtifacts(proposedBlock, payloads);

jframe · 2019-02-07T05:41:36Z

...ft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeArtifacts.java

@@ -47,16 +48,35 @@ public RoundChangeCertificate getRoundChangeCertificate() {
  public static RoundChangeArtifacts create(final Collection<RoundChange> roundChanges) {

    final Collection<SignedData<RoundChangePayload>> payloads =


can there be a test for this?

jframe · 2019-02-07T05:51:08Z

...ft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeArtifacts.java

-            .collect(Collectors.toList());
+        roundChanges.stream().map(RoundChange::getSignedPayload).collect(Collectors.toList());
+
+    Optional<RoundChange> newestRoundChange = empty();


you could probably use max or min change your compareNewRound to return an int so can be used as a comparator.

Something like

Optional<RoundChange> newestRoundChange = roundChanges.stream().max(RoundChangeArtifacts::compareNewRound); ... private static int compareNewRound( final RoundChange first, final RoundChange second) { final PreparedCertificate firstCert = first.getPreparedCertificate().get(); final PreparedCertificate secondCert = second.getPreparedCertificate().get(); return Integer.compare(secondCert.getProposalPayload().getPayload().getRoundIdentifier().getRoundNumber(), firstCert.getProposalPayload().getPayload().getRoundIdentifier().getRoundNumber()); }

jframe · 2019-02-07T05:55:01Z

.../java/tech/pegasys/pantheon/consensus/ibft/validation/ProposalBlockConsistencyValidator.java

+    }
+
+    if (proposedBlock.getHeader().getNumber()
+        != signedPayload.getPayload().getRoundIdentifier().getRoundNumber()) {


this looks like it's comparing the chain height to the round number instead of the sequence number

Ooops - now corrected.

rain-on requested review from jframe and CjHare February 5, 2019 02:11

IBFT2.1 crushed

0bc7691

rain-on force-pushed the new_ibft2 branch from 61b44fe to 0bc7691 Compare February 6, 2019 23:44

some test fixes

d4a436d

CjHare reviewed Feb 7, 2019

View reviewed changes

...sus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/MessageValidator.java Outdated Show resolved Hide resolved

rain-on commented Feb 7, 2019

View reviewed changes

consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/payload/MessageFactory.java Outdated Show resolved Hide resolved

rain-on commented Feb 7, 2019

View reviewed changes

tmohay added 2 commits February 7, 2019 14:35

post review

db1c489

Minor rework, now validate height of block

17f0c3e

jframe reviewed Feb 7, 2019

View reviewed changes

consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRound.java Outdated Show resolved Hide resolved

jframe reviewed Feb 7, 2019

View reviewed changes

tmohay and others added 12 commits February 7, 2019 17:08

fixed the G

a347b3a

added some unit tests and repaired height error

5e2c023

Repairs

7cfba38

Merge remote-tracking branch 'upstream/master' into my_ibft2

c9ef6d1

clean up RoundChangeArtefacts

ad507ff

Merge branch 'master' into new_ibft2

ca6832b

added test for RoundChangeArtifacts

1ae095f

repaired badly named proposalConsistencyChecker in NewRoundMsgValidator

0883073

fix errorprone

c80083a

more fixes

4b048ca

mod to use max

5a4d9ff

Merge branch 'master' into new_ibft2

45f2a5d

jframe approved these changes Feb 8, 2019

View reviewed changes

rain-on merged commit c044749 into PegaSysEng:master Feb 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IBFT 2.1 Message rework, piggybacking blocks on msgs. #769

IBFT 2.1 Message rework, piggybacking blocks on msgs. #769

rain-on commented Feb 5, 2019

rain-on commented Feb 5, 2019 •

edited

Loading

rain-on commented Feb 5, 2019

rain-on commented Feb 5, 2019 •

edited

Loading

rain-on commented Feb 5, 2019

rain-on Feb 7, 2019

rain-on Feb 7, 2019

jframe Feb 7, 2019

jframe Feb 7, 2019

rain-on Feb 8, 2019

jframe Feb 7, 2019

jframe Feb 7, 2019

rain-on Feb 7, 2019 •

edited

Loading

		@@ -47,16 +48,35 @@ public RoundChangeCertificate getRoundChangeCertificate() {
		public static RoundChangeArtifacts create(final Collection<RoundChange> roundChanges) {

		final Collection<SignedData<RoundChangePayload>> payloads =

IBFT 2.1 Message rework, piggybacking blocks on msgs. #769

IBFT 2.1 Message rework, piggybacking blocks on msgs. #769

Conversation

rain-on commented Feb 5, 2019

rain-on commented Feb 5, 2019 • edited Loading

rain-on commented Feb 5, 2019

rain-on commented Feb 5, 2019 • edited Loading

rain-on commented Feb 5, 2019

rain-on Feb 7, 2019

Choose a reason for hiding this comment

rain-on Feb 7, 2019

Choose a reason for hiding this comment

jframe Feb 7, 2019

Choose a reason for hiding this comment

jframe Feb 7, 2019

Choose a reason for hiding this comment

rain-on Feb 8, 2019

Choose a reason for hiding this comment

jframe Feb 7, 2019

Choose a reason for hiding this comment

jframe Feb 7, 2019

Choose a reason for hiding this comment

rain-on Feb 7, 2019 • edited Loading

Choose a reason for hiding this comment

rain-on commented Feb 5, 2019 •

edited

Loading

rain-on commented Feb 5, 2019 •

edited

Loading

rain-on Feb 7, 2019 •

edited

Loading